[PATCH] D110867: X86InstrInfo: Support immediates that are +1/-1 different in optimizeCompareInstr

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added inline comments.


================
Comment at: llvm/test/CodeGen/X86/jump_sign.ll:133
 
 ; If EFLAGS is live-out, we can't remove cmp if there exists
 ; a swapped sub.
----------------
xbolva00 wrote:
> cmp now  removed
Good catch. So:
- I believe the comment to be outdated:
   - When you look at r159888 adding the test: https://github.com/llvm/llvm-project/blob/bb36074047985cb476cadda464447b634972be2a/llvm/test/CodeGen/X86/jump_sign.ll#L110 the intention was that the cmp behind the sub is preserved. The single `CHECK` however doesn't really capture this and can match the `cmp` in bb.0 and the one in bb.1
   - I believe that SelectionDAG pattern matching improved in the meantime so nowadays the cmp is eliminated at that stage.
- The new version of the code is correct; you see that previously it used a `cmpl ecx, edx`, but `edx and eax` contain the same value and the sub in `bb.0` the only predecessor of `bb.1` is just the reverse operation and will produce the reverse flags. So we can indeed remove the cmp when we reverse the user flags `cmovle -> cmovl`.
- I accidentally put this test change onto the wrong commit in the stack. The test actually changes in D110862.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110867