[PATCH] D43349: [InstCombine] Make SimplifyDemandedUseBits handle PhiNode

[Rong Xu via Phabricator via llvm-commits]

xur added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp:479
       if (cast<LShrOperator>(I)->isExact())
         DemandedMaskIn.setLowBits(ShiftAmt);
 
----------------
craig.topper wrote:
> xur wrote:
> > craig.topper wrote:
> > > DemandedMaskIn isn't used after this.
> > I actually did not understand why DemandedMaskIn is reset here. So I moved it down (after processed the operand 0).
> > 
> > To me, since we  are doing the do the shift right, ShiftAmt of the lowbits is not needed (whatever value it might be).
> > The original code seems to say these lowbits are needed. 
> > 
> > If the lowbits in DemandMaskin are indeed needed, I would have to set the Known.zero and check Known.zero in AND instruction.
> > 
> If we dont' preserve the bits in the input to the shift, then the exact flag could become incorrect. I'm not sure why we would prefer to preserve the bits rather than clear the exact flag.
You are right! Thanks for the explanation.  If the operand marked as exact, it's unsafe the remove the AND operation. 
 This exact in lshr seems to be insert by instcombine (with the presence of AND).
It won's apply  to my original c++ programs. 

Changed the test case as suggested by dberlin.



https://reviews.llvm.org/D43349