resolve review comments

Author: hassnaaHamdi

llvm/lib/CodeGen/CodeGenPrepare.cpp

@@ -338,6 +338,10 @@ class CodeGenPrepare {
   /// Keep track of instructions removed during promotion.
   SetOfInstrs RemovedInsts;
 
+  /// Keep track of seen mul_with_overflow intrinsics to avoid
+  // reprocessing them.
+  DenseMap<Instruction *, bool> SeenMulWithOverflowInstrs;
+
   /// Keep track of sext chains based on their initial value.
   DenseMap<Value *, Instruction *> SeenChainsForSExt;
 
@@ -433,6 +437,8 @@ class CodeGenPrepare {
   bool optimizeGatherScatterInst(Instruction *MemoryInst, Value *Ptr);
   bool optimizeUMulWithOverflow(Instruction *I);
   bool optimizeSMulWithOverflow(Instruction *I);
+  bool optimizeMulWithOverflow(Instruction *I, bool IsSigned,
+                               ModifyDT &ModifiedDT);
   bool optimizeInlineAsmInst(CallInst *CS);
   bool optimizeCallInst(CallInst *CI, ModifyDT &ModifiedDT);
   bool optimizeExt(Instruction *&I);
@@ -774,6 +780,7 @@ bool CodeGenPrepare::_run(Function &F) {
     verifyBFIUpdates(F);
 #endif
 
+  SeenMulWithOverflowInstrs.clear();
   return EverMadeChange;
 }
 
@@ -2781,9 +2788,9 @@ bool CodeGenPrepare::optimizeCallInst(CallInst *CI, ModifyDT &ModifiedDT) {
       }
       return false;
     case Intrinsic::umul_with_overflow:
-      return optimizeUMulWithOverflow(II);
+      return optimizeMulWithOverflow(II, /*IsSigned=*/false, ModifiedDT);
     case Intrinsic::smul_with_overflow:
-      return optimizeSMulWithOverflow(II);
+      return optimizeMulWithOverflow(II, /*IsSigned=*/true, ModifiedDT);
     }
 
     SmallVector<Value *, 2> PtrOps;
@@ -6395,122 +6402,20 @@ bool CodeGenPrepare::optimizeGatherScatterInst(Instruction *MemoryInst,
   return true;
 }
 
-// Rewrite the umul_with_overflow intrinsic by checking if both of the
+// Rewrite the mul_with_overflow intrinsic by checking if both of the
 // operands' value range is within the legal type. If so, we can optimize the
 // multiplication algorithm. This code is supposed to be written during the step
 // of type legalization, but given that we need to reconstruct the IR which is
 // not doable there, we do it here.
-bool CodeGenPrepare::optimizeUMulWithOverflow(Instruction *I) {
+bool CodeGenPrepare::optimizeMulWithOverflow(Instruction *I, bool IsSigned,
+                                             ModifyDT &ModifiedDT) {
   // Enable this optimization only for aarch64.
   if (!TLI->getTargetMachine().getTargetTriple().isAArch64())
     return false;
-  if (TLI->getTypeAction(
-          I->getContext(),
-          TLI->getValueType(*DL, I->getType()->getContainedType(0))) !=
-      TargetLowering::TypeExpandInteger)
-    return false;
-
-  Value *LHS = I->getOperand(0);
-  Value *RHS = I->getOperand(1);
-  auto *Ty = LHS->getType();
-  unsigned VTBitWidth = Ty->getScalarSizeInBits();
-  unsigned VTHalfBitWidth = VTBitWidth / 2;
-  auto *LegalTy = IntegerType::getIntNTy(I->getContext(), VTHalfBitWidth);
-
-  // Skip the optimization if the type with HalfBitWidth is not legal for the
-  // target.
-  if (TLI->getTypeAction(I->getContext(), TLI->getValueType(*DL, LegalTy)) !=
-      TargetLowering::TypeLegal)
+  // If we have already seen this instruction, don't process it again.
+  if (!SeenMulWithOverflowInstrs.insert(std::make_pair(I, true)).second)
     return false;
 
-  I->getParent()->setName("overflow.res");
-  auto *OverflowResBB = I->getParent();
-  auto *OverflowoEntryBB =
-      I->getParent()->splitBasicBlock(I, "overflow.entry", /*Before*/ true);
-  BasicBlock *NoOverflowBB = BasicBlock::Create(
-      I->getContext(), "overflow.no", I->getFunction(), OverflowResBB);
-  BasicBlock *OverflowBB = BasicBlock::Create(I->getContext(), "overflow",
-                                              I->getFunction(), OverflowResBB);
-  // new blocks should be:
-  //  entry:
-  //    (lhs_lo ne lhs_hi) || (rhs_lo ne rhs_hi) ? overflow, overflow_no
-
-  //  overflow_no:
-  //  overflow:
-  //  overflow.res:
-  //------------------------------------------------------------------------------
-  // BB overflow.entry:
-  // get Lo and Hi of RHS & LHS:
-  IRBuilder<> Builder(OverflowoEntryBB->getTerminator());
-  auto *LoRHS = Builder.CreateTrunc(RHS, LegalTy, "lo.rhs.trunc");
-  auto *ShrHiRHS = Builder.CreateLShr(RHS, VTHalfBitWidth, "rhs.lsr");
-  auto *HiRHS = Builder.CreateTrunc(ShrHiRHS, LegalTy, "hi.rhs.trunc");
-
-  auto *LoLHS = Builder.CreateTrunc(LHS, LegalTy, "lo.lhs.trunc");
-  auto *ShrHiLHS = Builder.CreateLShr(LHS, VTHalfBitWidth, "lhs.lsr");
-  auto *HiLHS = Builder.CreateTrunc(ShrHiLHS, LegalTy, "hi.lhs.trunc");
-
-  auto *CmpLHS = Builder.CreateCmp(ICmpInst::ICMP_NE, HiLHS,
-                                   ConstantInt::getNullValue(LegalTy));
-  auto *CmpRHS = Builder.CreateCmp(ICmpInst::ICMP_NE, HiRHS,
-                                   ConstantInt::getNullValue(LegalTy));
-  auto *Or = Builder.CreateOr(CmpLHS, CmpRHS, "or.lhs.rhs");
-  Builder.CreateCondBr(Or, OverflowBB, NoOverflowBB);
-  OverflowoEntryBB->getTerminator()->eraseFromParent();
-
-  //------------------------------------------------------------------------------
-  // BB overflow.no:
-  Builder.SetInsertPoint(NoOverflowBB);
-  auto *ExtLoLHS = Builder.CreateZExt(LoLHS, Ty, "lo.lhs.ext");
-  auto *ExtLoRHS = Builder.CreateZExt(LoRHS, Ty, "lo.rhs.ext");
-  auto *Mul = Builder.CreateMul(ExtLoLHS, ExtLoRHS, "mul.no.overflow");
-  Builder.CreateBr(OverflowResBB);
-
-  //------------------------------------------------------------------------------
-  // BB overflow.res:
-  Builder.SetInsertPoint(OverflowResBB, OverflowResBB->getFirstInsertionPt());
-  auto *PHINode1 = Builder.CreatePHI(Ty, 2);
-  PHINode1->addIncoming(Mul, NoOverflowBB);
-  auto *PHINode2 =
-      Builder.CreatePHI(IntegerType::getInt1Ty(I->getContext()), 2);
-  PHINode2->addIncoming(ConstantInt::getFalse(I->getContext()), NoOverflowBB);
-
-  StructType *STy = StructType::get(
-      I->getContext(), {Ty, IntegerType::getInt1Ty(I->getContext())});
-  Value *StructValOverflowRes = PoisonValue::get(STy);
-  StructValOverflowRes =
-      Builder.CreateInsertValue(StructValOverflowRes, PHINode1, {0});
-  StructValOverflowRes =
-      Builder.CreateInsertValue(StructValOverflowRes, PHINode2, {1});
-  // Before moving the mul.overflow intrinsic to the overflowBB, replace all its
-  // uses by StructValOverflowRes.
-  I->replaceAllUsesWith(StructValOverflowRes);
-  I->removeFromParent();
-
-  // BB overflow:
-  I->insertInto(OverflowBB, OverflowBB->end());
-  Builder.SetInsertPoint(OverflowBB, OverflowBB->end());
-  auto *MulOverflow = Builder.CreateExtractValue(I, {0}, "mul.overflow");
-  auto *OverflowFlag = Builder.CreateExtractValue(I, {1}, "overflow.flag");
-  Builder.CreateBr(OverflowResBB);
-
-  // Add The Extracted values to the PHINodes in the overflow.res block.
-  PHINode1->addIncoming(MulOverflow, OverflowBB);
-  PHINode2->addIncoming(OverflowFlag, OverflowBB);
-
-  // return false to stop reprocessing the function.
-  return false;
-}
-
-// Rewrite the smul_with_overflow intrinsic by checking if both of the
-// operands' value range is within the legal type. If so, we can optimize the
-// multiplication algorithm. This code is supposed to be written during the step
-// of type legalization, but given that we need to reconstruct the IR which is
-// not doable there, we do it here.
-bool CodeGenPrepare::optimizeSMulWithOverflow(Instruction *I) {
-  // Enable this optimization only for aarch64.
-  if (!TLI->getTargetMachine().getTargetTriple().isAArch64())
-    return false;
   if (TLI->getTypeAction(
           I->getContext(),
           TLI->getValueType(*DL, I->getType()->getContainedType(0))) !=
@@ -6519,75 +6424,93 @@ bool CodeGenPrepare::optimizeSMulWithOverflow(Instruction *I) {
 
   Value *LHS = I->getOperand(0);
   Value *RHS = I->getOperand(1);
-  auto *Ty = LHS->getType();
+  Type *Ty = LHS->getType();
   unsigned VTBitWidth = Ty->getScalarSizeInBits();
   unsigned VTHalfBitWidth = VTBitWidth / 2;
-  auto *LegalTy = IntegerType::getIntNTy(I->getContext(), VTHalfBitWidth);
+  IntegerType *LegalTy =
+      IntegerType::getIntNTy(I->getContext(), VTHalfBitWidth);
 
   // Skip the optimization if the type with HalfBitWidth is not legal for the
   // target.
   if (TLI->getTypeAction(I->getContext(), TLI->getValueType(*DL, LegalTy)) !=
       TargetLowering::TypeLegal)
     return false;
 
+  // Make sure that the I->getType() is a struct type with two elements.
+  if (!I->getType()->isStructTy() || I->getType()->getStructNumElements() != 2)
+    return false;
+
   I->getParent()->setName("overflow.res");
-  auto *OverflowResBB = I->getParent();
-  auto *OverflowoEntryBB =
+  BasicBlock *OverflowResBB = I->getParent();
+  BasicBlock *OverflowoEntryBB =
       I->getParent()->splitBasicBlock(I, "overflow.entry", /*Before*/ true);
   BasicBlock *NoOverflowBB = BasicBlock::Create(
       I->getContext(), "overflow.no", I->getFunction(), OverflowResBB);
   BasicBlock *OverflowBB = BasicBlock::Create(I->getContext(), "overflow",
                                               I->getFunction(), OverflowResBB);
   // new blocks should be:
   //  entry:
-  //    (lhs_lo ne lhs_hi) || (rhs_lo ne rhs_hi) ? overflow, overflow_no
+  //    if signed:
+  //      (lhs_lo ^ lhs_hi) || (rhs_lo ^ rhs_hi) ? overflow, overflow_no
+  //    else:
+  //      (lhs_hi != 0) || (rhs_hi != 0) ? overflow, overflow_no
 
   //  overflow_no:
   //  overflow:
   //  overflow.res:
 
-  //------------------------------------------------------------------------------
+  // ----------------------------
   // BB overflow.entry:
-  // get Lo and Hi of RHS & LHS:
+  // get Lo and Hi of LHS & RHS:
   IRBuilder<> Builder(OverflowoEntryBB->getTerminator());
-  auto *LoRHS = Builder.CreateTrunc(RHS, LegalTy, "lo.rhs");
-  auto *SignLoRHS =
-      Builder.CreateAShr(LoRHS, VTHalfBitWidth - 1, "sign.lo.rhs");
-  auto *HiRHS = Builder.CreateLShr(RHS, VTHalfBitWidth, "rhs.lsr");
+  Value *LoLHS = Builder.CreateTrunc(LHS, LegalTy, "lo.lhs");
+  Value *HiLHS = Builder.CreateLShr(LHS, VTHalfBitWidth, "lhs.lsr");
+  HiLHS = Builder.CreateTrunc(HiLHS, LegalTy, "hi.lhs");
+  Value *LoRHS = Builder.CreateTrunc(RHS, LegalTy, "lo.rhs");
+  Value *HiRHS = Builder.CreateLShr(RHS, VTHalfBitWidth, "rhs.lsr");
   HiRHS = Builder.CreateTrunc(HiRHS, LegalTy, "hi.rhs");
 
-  auto *LoLHS = Builder.CreateTrunc(LHS, LegalTy, "lo.lhs");
-  auto *SignLoLHS =
-      Builder.CreateAShr(LoLHS, VTHalfBitWidth - 1, "sign.lo.lhs");
-  auto *HiLHS = Builder.CreateLShr(LHS, VTHalfBitWidth, "lhs.lsr");
-  HiLHS = Builder.CreateTrunc(HiLHS, LegalTy, "hi.lhs");
-  // xor(HiLHS, SignLoLHS) false -> no overflow
-  // xor(HiRHS, SignLoRHS) false -> no overflow
-  // if either of the above is true, then overflow.
-  // auto *CmpLHS = Builder.CreateCmp(ICmpInst::ICMP_NE, HiLHS, SignLoLHS);
-  auto *XorLHS = Builder.CreateXor(HiLHS, SignLoLHS);
-  auto *XorRHS = Builder.CreateXor(HiRHS, SignLoRHS);
-  // auto *CmpRHS = Builder.CreateCmp(ICmpInst::ICMP_NE, HiRHS, SignLoRHS);
-  auto *Or = Builder.CreateOr(XorLHS, XorRHS, "or.lhs.rhs");
-  auto *Cmp = Builder.CreateCmp(ICmpInst::ICMP_EQ, Or,
-                                ConstantInt::get(Or->getType(), 1));
-  Builder.CreateCondBr(Cmp, OverflowBB, NoOverflowBB);
+  Value *IsAnyBitTrue;
+  if (IsSigned) {
+    Value *SignLoLHS =
+        Builder.CreateAShr(LoLHS, VTHalfBitWidth - 1, "sign.lo.lhs");
+    Value *SignLoRHS =
+        Builder.CreateAShr(LoRHS, VTHalfBitWidth - 1, "sign.lo.rhs");
+    Value *XorLHS = Builder.CreateXor(HiLHS, SignLoLHS);
+    Value *XorRHS = Builder.CreateXor(HiRHS, SignLoRHS);
+    Value *Or = Builder.CreateOr(XorLHS, XorRHS, "or.lhs.rhs");
+    IsAnyBitTrue = Builder.CreateCmp(ICmpInst::ICMP_EQ, Or,
+                                     ConstantInt::get(Or->getType(), 1));
+  } else {
+    Value *CmpLHS = Builder.CreateCmp(ICmpInst::ICMP_NE, HiLHS,
+                                      ConstantInt::getNullValue(LegalTy));
+    Value *CmpRHS = Builder.CreateCmp(ICmpInst::ICMP_NE, HiRHS,
+                                      ConstantInt::getNullValue(LegalTy));
+    IsAnyBitTrue = Builder.CreateOr(CmpLHS, CmpRHS, "or.lhs.rhs");
+  }
+
+  Builder.CreateCondBr(IsAnyBitTrue, OverflowBB, NoOverflowBB);
   OverflowoEntryBB->getTerminator()->eraseFromParent();
 
-  //------------------------------------------------------------------------------
   // BB overflow.no:
   Builder.SetInsertPoint(NoOverflowBB);
-  auto *ExtLoLHS = Builder.CreateSExt(LoLHS, Ty, "lo.lhs.ext");
-  auto *ExtLoRHS = Builder.CreateSExt(LoRHS, Ty, "lo.rhs.ext");
-  auto *Mul = Builder.CreateMul(ExtLoLHS, ExtLoRHS, "mul.no.overflow");
+  Value *ExtLoLHS, *ExtLoRHS;
+  if (IsSigned) {
+    ExtLoLHS = Builder.CreateSExt(LoLHS, Ty, "lo.lhs.ext");
+    ExtLoRHS = Builder.CreateSExt(LoRHS, Ty, "lo.rhs.ext");
+  } else {
+    ExtLoLHS = Builder.CreateZExt(LoLHS, Ty, "lo.lhs.ext");
+    ExtLoRHS = Builder.CreateZExt(LoRHS, Ty, "lo.rhs.ext");
+  }
+
+  Value *Mul = Builder.CreateMul(ExtLoLHS, ExtLoRHS, "mul.no.overflow");
   Builder.CreateBr(OverflowResBB);
 
-  //------------------------------------------------------------------------------
   // BB overflow.res:
   Builder.SetInsertPoint(OverflowResBB, OverflowResBB->getFirstInsertionPt());
-  auto *PHINode1 = Builder.CreatePHI(Ty, 2);
+  PHINode *PHINode1 = Builder.CreatePHI(Ty, 2);
   PHINode1->addIncoming(Mul, NoOverflowBB);
-  auto *PHINode2 =
+  PHINode *PHINode2 =
       Builder.CreatePHI(IntegerType::getInt1Ty(I->getContext()), 2);
   PHINode2->addIncoming(ConstantInt::getFalse(I->getContext()), NoOverflowBB);
 
@@ -6606,16 +6529,16 @@ bool CodeGenPrepare::optimizeSMulWithOverflow(Instruction *I) {
   // BB overflow:
   I->insertInto(OverflowBB, OverflowBB->end());
   Builder.SetInsertPoint(OverflowBB, OverflowBB->end());
-  auto *MulOverflow = Builder.CreateExtractValue(I, {0}, "mul.overflow");
-  auto *OverflowFlag = Builder.CreateExtractValue(I, {1}, "overflow.flag");
+  Value *MulOverflow = Builder.CreateExtractValue(I, {0}, "mul.overflow");
+  Value *OverflowFlag = Builder.CreateExtractValue(I, {1}, "overflow.flag");
   Builder.CreateBr(OverflowResBB);
 
   // Add The Extracted values to the PHINodes in the overflow.res block.
   PHINode1->addIncoming(MulOverflow, OverflowBB);
   PHINode2->addIncoming(OverflowFlag, OverflowBB);
 
-  // return false to stop reprocessing the function.
-  return false;
+  ModifiedDT = ModifyDT::ModifyBBDT;
+  return true;
 }
 
 /// If there are any memory operands, use OptimizeMemoryInst to sink their