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

[Alex Bradbury via Phabricator via llvm-commits]

asb added a comment.

In D98932#2663612 <https://reviews.llvm.org/D98932#2663612>, @asb wrote:

> It would definitely be good to get a test case for this as Fraser says.
>
> I appreciate it's a bit hard to implement an alternate fix that doesn't involve the risk of regressing other backends, but it's not ideal to remove a call to a hook that really RISC-V should be setting (per the description of setHasMultipleConditionRegisters, this should be set for RISC-V).
>
> This is a good point fix for the immediate problem, but the concern is just that future optimiser changes means there are more issues or missed optimisation opportunities down the road.
>
> If we did want to do this, probably worth adding comments to explain that we'd like to setHasMultipleConditionRegisters ideally, but are choosing not to due to codegen regressions.

Hi Philipp - did you have any thoughts on whether an alternate fix may be viable (even if it involves adding a new hook)? As noted above, claiming not to have multiple condition registers when we do (at least according to the setHasMultipleConditionRegisters doc comment) isn't ideal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98932