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

[Mingming Liu via Phabricator via llvm-commits]

mingmingl marked an inline comment as done.
mingmingl added inline comments.


================
Comment at: llvm/test/CodeGen/X86/peephole-test-after-add.mir:23
+    %5 = load i64, ptr %0, align 8
+    %6 = and i64 %5, 3
+    %7 = icmp slt i64 %6, 0
----------------
craig.topper wrote:
> mingmingl wrote:
> > craig.topper wrote:
> > > davidxl wrote:
> > > > The and instruction clears SF and upper  bits of the value which means the test instruction later will also produce SF == 0. In other words, in this case,  not only the test instruction, but also the CMOVErr can be optimized into a copy.  
> > > > 
> > > > (this can be looked at in a follow up patch if it is the case).
> > > Do we not figure that out in SelectionDAG?
> > > The and instruction clears SF and upper bits of the value which means the test instruction later will also produce SF == 0. In other words, in this case, not only the test instruction, but also the CMOVErr can be optimized into a copy.
> > 
> > > (this can be looked at in a follow up patch if it is the case).
> > 
> > Added a FIXME around line 1052 (inside `findRedundantFlagInstr`) to follow up on this.
> > 
> > > Do we not figure that out in SelectionDAG?
> > 
> > Not sure if we could figure it out in SelectionDAG step. There is `TargetInstrInfo::optimizeSelect` virtual method for peephole-opt, and an implementation (`ARMBaseInstrInfo::optimizeSelect`) [1]. Will look into which pass to put this optimization later.
> > 
> > [1] https://github.com/llvm/llvm-project/blob/0353c2c996c5863463c356de97c9852f9330ed11/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp#L2321
> I thought DAGCombiner for ISD::SETCC should be able to figure out that an AND with 3 guarantees the sign bit is 0, but it appears it does not. InstCombine does optimize it though.
Got it. DAGCombiner sounds a right place to look at a sequence of instructions and optimize.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124118