Arm64: Skip redundant hwintrinsic float movs (#58954) #75202
Conversation
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsFor every FMOV emitted by the hardware intrinsics that is currently marked as not skippable: if the src and dest registers are the same and the types match, then the instruction will have no effect and can be safely marked as skippable.
|
|
static float Lerp(float v0, float v1, float t) => Before: With patch: |
|
Another way would be to do the check inside emitIns_Mov and get rid of canSkip, but that'd be a little messier to get all the logic right. Alternatively, avoid calling emitIns_Mov() when the check passes in hw intrinsic code. |
kunalspathak
left a comment
There was a problem hiding this comment.
Changes LGTM. Can you fix the format errors and I can retrigger the runtime CI? I don't think the failures are related to your changes.
For every FMOV emitted by the hardware intrinsics that is currently marked as not skippable: if the src and dest registers are the same and the types match, then the instruction will have no effect and can be safely marked as skippable.
caa83db to
89de55a
Compare
|
Also cc @tannergooding |
| assert(intrin.baseType == TYP_DOUBLE); | ||
| assert(GetEmitter()->IsMovInstruction(ins)); | ||
| GetEmitter()->emitIns_Mov(ins, emitSize, targetReg, op1Reg, /* canSkip */ false, opt); | ||
| bool canSkip = ((targetReg == op1Reg) && (intrin.baseType == intrin.op1->gtType)); |
There was a problem hiding this comment.
Same here, this one should always be skippable since DuplicateToVector64(double) is a "nop" (due to double being 64-bits and already in a SIMD register).
There was a problem hiding this comment.
It would be good to confirm what path can hit this, however. Is it a synthesized node in lowering or elsewhere?
We don't have a managed size equivalent here.
There was a problem hiding this comment.
It would be good to confirm what path can hit this, however. Is it a synthesized node in lowering or elsewhere?
This was the smallest example I found in spmi replay:
dump-225213.txt
(The Assert hit was just added to catch this)
|
All updated as suggested - which I think changed every line in the patch. |
f6f18a6 to
80e7d14
Compare
For every FMOV emitted by the hardware intrinsics that is currently marked as not skippable: if the src and dest registers are the same and the types match, then the instruction will have no effect and can be safely marked as skippable.
Fixes: #58954