[PATCH] D25200: [InstCombine] New opportunities for FoldAndOfICmp and FoldXorOfICmp
Ehsan Amiri via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 29 13:08:54 PST 2016
amehsan marked an inline comment as not done.
amehsan added inline comments.
================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:850
+
+ if (RHSCst->isZero() && LHSCst->isZero()) {
+
----------------
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.
================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:758-766
+ auto *Inst = dyn_cast<Instruction>(LHS);
+
+ if (!Inst || Inst->getOpcode() != Instruction::And)
+ return BGC;
+
+ // TODO Currently this does not work for vectors.
+ auto *Mask = dyn_cast<ConstantInt>(Inst->getOperand(1));
----------------
spatel wrote:
> Simplify?
> ConstantInt *Mask;
> if (!match(LHS, m_And(m_Value(), m_ConstantInt(Mask))))
> return BGC;
>
Sure.
================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2817
+ // and we know both A and B are either 8 (power of 2) or 0
+ // we can simplify to (icmp ne A, B)
+ if (Value *Res = FoldXorOfICmps(LHS, RHS))
----------------
spatel wrote:
> Period at end of sentence.
Sure.
https://reviews.llvm.org/D25200
More information about the llvm-commits
mailing list