[PATCH] D124118: [Peephole-Opt] For one kind of test-after-add pattern, eliminates test if it's correct to do so.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:970
 
+// Returns true if SF is known to be zero after AndInstr executes.
+inline static bool isSFZero(const MachineInstr &AndInstr) {
----------------
mingmingl wrote:
> craig.topper wrote:
> > I don't know if this is necessary. If the SUBREG_TO_REG was used, the sign bit of the full 64 bit value was provably zero during SelectionDAG. I hope any branch, setcc, cmov would have been folded away based on it.
> > 
> > Can't we just set NoSignFlag=true?
> `isSFZero` is used to tell the difference of following two scenarios, and (if I read correctly) `NoSignFlag` (as well as `ClearsOverflowFlag`) indicate whether the EFLAG bit is set (as opposed to the value of bit), so for `AND` operation, we want to set `NoSignFlag` to false.
> 
> 1) 
> 
> ```
> AND32ri8 reg, 0x9  # signed bit is 1, so after sign extension, the SF bit could be 1 or 0 depending on reg value
> SUBREG_TO_REG 0, extended_reg, reg , sub_32bit
> TEST64rr extended_reg, extended_reg,  # SF bit is 0 since higher 32 bit of extended_reg is 0
> ```
> 
> 2) 
> 
> ```
> AND32ri8 reg, 0x7  # signed bit is 0, so after sign extension, the SF bit is known to be 0
> SUBREG_TO_REG 0, extended_reg, reg , sub_32bit
> TEST64rr extended_reg, extended_reg,  # SF bit is 0 since higher 32 bit of extended_reg is 0
> ```
> 
> We want to optimize away `test` for 2) but keep `test` for 1).
> 
> Let me know if I miss anything!
If the instructions that use the flags from the TEST don't look at the SF flag then it doesn't matter. Setting `NoSignFlag = true` will disable the optimization if the SF flag is used. Your motivating test case doesn't use the sign flag so this should be sufficient.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124118