[PATCH] D25200: [InstCombine] New opportunities for FoldAndOfICmp and FoldXorOfICmp

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:850
+
+  if (RHSCst->isZero() && LHSCst->isZero()) {
+
----------------
amehsan wrote:
> spatel wrote:
> > amehsan wrote:
> > > spatel wrote:
> > > > Can we check that RHSCst == LHSCst and remove the getBitWidth checks below?
> > > See the respond to a similar comment below.
> > Sorry for not seeing it, but I don't know how it's possible that:
> >   (*BGC1.Mask == *BGC2.Mask) && (BGC1.Mask->getBitWidth() != BGC2.Mask->getBitWidth())
> > 
> > Can you add a test case to show where that happens?
> > 
> > To be clear, my proposal is to change the initial check:
> >   if (RHSCst->isZero() && LHSCst->isZero())
> > to:
> >   if (RHSCst->isZero() && LHSCst == RHSCst)
> > 
> > And then remove the bit width check from the later 'if' statement. The operand types of icmp and xor/and must match, so I thought that would be sufficient?
> operator== that is called with (*BGC1.Mask == *BGC2.Mask) has an assertion that the two masks has the same bitwidth. IIUC, if we want to do LHSCst == RHSCst the same operator== is called, and so we would need to check the bit widths of RHSCst and LHSCst. So overall it does not make a difference.  
*BGC1.Mask == *BGC2.Mask is the APInt operator== and yes, it asserts on bitwidth.
LHSCst == RHSCst is just an equality check of 2 pointers. Ie, I think there can only be one '0' value of a given bitwidth.



https://reviews.llvm.org/D25200