[InstCombine] [NFC] refector foldXorOfICmps

[RKSimon]

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

3000

If the if() below fails - doesn't this leave the LHS0/LHS1 operands in a commuted state further down?

[Chenbing.Zheng]

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

3000

(LHS0 == RHS1 && LHS1 == RHS0) is true, means (LHS0 == RHS0 && LHS1 == RHS1) must be true.
In other words, if do swap, we must go to the next conditional branch.

I think this is consistent with the original logic.

[Chenbing.Zheng]

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

3000

Correct
(LHS0 == RHS1 && LHS1 == RHS0) is true and after swap(LHS0, LHS1), means (LHS0 == RHS0 && LHS1 == RHS1) must be true.

[nikic]

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

3000

Aren't you missing a PredL = ICmpInst::getSwappedPredicate(PredL) here?

[Chenbing.Zheng]

address nikic's comment

[Chenbing.Zheng]

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

3000

Thanks and done.

[nikic]

LGTM

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

static Instruction *foldXorToXor(BinaryOperator &I,
   return nullptr;
 }
 
 Value *InstCombinerImpl::foldXorOfICmps(ICmpInst *LHS, ICmpInst *RHS,
                                         BinaryOperator &I) {
   assert(I.getOpcode() == Instruction::Xor && I.getOperand(0) == LHS &&
          I.getOperand(1) == RHS && "Should be 'xor' with these operands");
 
-  if (predicatesFoldable(LHS->getPredicate(), RHS->getPredicate())) {
-    if (LHS->getOperand(0) == RHS->getOperand(1) &&
-        LHS->getOperand(1) == RHS->getOperand(0))
-      LHS->swapOperands();
-    if (LHS->getOperand(0) == RHS->getOperand(0) &&
-        LHS->getOperand(1) == RHS->getOperand(1)) {
+  ICmpInst::Predicate PredL = LHS->getPredicate(), PredR = RHS->getPredicate();
+  Value *LHS0 = LHS->getOperand(0), *LHS1 = LHS->getOperand(1);
+  Value *RHS0 = RHS->getOperand(0), *RHS1 = RHS->getOperand(1);
+
+  if (predicatesFoldable(PredL, PredR)) {
+    if (LHS0 == RHS1 && LHS1 == RHS0)
+      std::swap(LHS0, LHS1);
+    if (LHS0 == RHS0 && LHS1 == RHS1) {
       // (icmp1 A, B) ^ (icmp2 A, B) --> (icmp3 A, B)
-      Value *Op0 = LHS->getOperand(0), *Op1 = LHS->getOperand(1);
-      unsigned Code =
-          getICmpCode(LHS->getPredicate()) ^ getICmpCode(RHS->getPredicate());
+      unsigned Code = getICmpCode(PredL) ^ getICmpCode(PredR);
       bool IsSigned = LHS->isSigned() || RHS->isSigned();
-      return getNewICmpValue(Code, IsSigned, Op0, Op1, Builder);
+      return getNewICmpValue(Code, IsSigned, LHS0, LHS1, Builder);
     }
   }
 
   // TODO: This can be generalized to compares of non-signbits using
   // decomposeBitTestICmp(). It could be enhanced more by using (something like)
   // foldLogOpOfMaskedICmps().
-  ICmpInst::Predicate PredL = LHS->getPredicate(), PredR = RHS->getPredicate();
-  Value *LHS0 = LHS->getOperand(0), *LHS1 = LHS->getOperand(1);
-  Value *RHS0 = RHS->getOperand(0), *RHS1 = RHS->getOperand(1);
   if ((LHS->hasOneUse() || RHS->hasOneUse()) &&
       LHS0->getType() == RHS0->getType() &&
       LHS0->getType()->isIntOrIntVectorTy()) {
     // (X > -1) ^ (Y > -1) --> (X ^ Y) < 0
     // (X <  0) ^ (Y <  0) --> (X ^ Y) < 0
     if ((PredL == CmpInst::ICMP_SGT && match(LHS1, m_AllOnes()) &&
          PredR == CmpInst::ICMP_SGT && match(RHS1, m_AllOnes())) ||
         (PredL == CmpInst::ICMP_SLT && match(LHS1, m_Zero()) &&