jayfoad wrote:

I've taken another look at this. The patch does not show any benefit from 
running another `SIFoldOperands` pass _after_ `SIShrinkInstructions` per se; 
you get exactly the same results (modulo a couple of add instructions that have 
their operands commuted differently) if you put the second `SIFoldOperands` run 
_before_ `SIShrinkInstructions` instead.

In other words `SIFoldOperands` is not idempotent, and the reason for the that 
seems to be:

> And the reason it only happens for some SUBREV instructions is even more 
> convoluted. It's because SIFoldOperands will sometimes shrink 
> V_SUB_CO_U32_e64 to V_SUBREV_CO_U32_e32 even it does not manage to fold 
> anything into it. This does seem wrong and is probably worth a closer look.

This goes back to https://reviews.llvm.org/D51345. Notice how the code that was 
added to `updateOperand` does the shrinking but does not actually do any 
folding; it returns before we get to 
`Old.ChangeToImmediate`/`Old.substVirtReg`. A second run of `SIFoldOperands` 
will see the shrunk instruction and fold into it.

https://github.com/llvm/llvm-project/pull/67878
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to