[PATCH] D98932: [RISCV] Don't call setHasMultipleConditionRegisters(), so icmp is sunk

[Paolo Savini via Phabricator via llvm-commits]

PaoloS added a comment.

I quite agree that `hasMultipleConditionRegisters` seems to be intended for targets like PowerPC (only when it treats the 32 bits of the condition register as separate entities).
It also seems to me that the hook was added to RISCV as a simpler way make the RISCV backend lower select into more more logic instructions and fewer branches, as explained here: https://reviews.llvm.org/D79268
This wasn't probably very descriptive of the condition registers situation of RISCV that differs from the one of PowerPC (and AMDGPU) for which the hook was made for.
As already said, due to a previous commit, the hook `hasMultipleConditionRegisters` also influences (prevents) the sinking of compare instructions (`sinkCmpExpression`) and I found myself that by sinking them we get better code size.
I ran the benchmarks of Embench (https://github.com/embench/embench-iot) with a custom option I added (for proof of concept: https://reviews.llvm.org/D105620) and saw that by turning off that hook we achieve better code size.
I think that the idea to create another hook that is specific to the lowering of the select instructions is good. If that's preferable to overriding shouldNormalizeToSelectSequence to just return false.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98932/new/

https://reviews.llvm.org/D98932