[ConstraintElim] Store the triple Pred + LHS + RHS in ReproducerEntry instead of CmpInst + Not

[dtcxzyw]

Summary

This patch represents a condition with Pred + LHS + RHS in ReproducerEntry instead of CmpInst + Not.
It avoids creating temporary ICmpInsts in D155412.

[nikic]

LGTM

llvm/lib/Transforms/Scalar/ConstraintElimination.cpp

997

Add an icmp prefix to make it the same as the previous output?

999

You can pass false as the second arg here to avoid printing the type again, matching how icmp is usually printed.

[dtcxzyw]

Address comments

[fhahn]

LGTM, thanks for the follow-up!

llvm/lib/Transforms/Scalar/ConstraintElimination.cpp

void State::addInfoFor(BasicBlock &BB) {
   if (canAddSuccessor(BB, Br->getSuccessor(1)))
     WorkList.emplace_back(
         FactOrCheck::getFact(DT.getNode(Br->getSuccessor(1)), CmpI, true));
 }
 
 namespace {
 /// Helper to keep track of a condition and if it should be treated as negated
 /// for reproducer construction.
+/// Pred == Predicate::BAD_ICMP_PREDICATE indicates that this entry is a
+/// placeholder to keep the ReproducerCondStack in sync with DFSInStack.
 struct ReproducerEntry {
-  CmpInst *Cond;
-  bool IsNot;
+  ICmpInst::Predicate Pred;
+  Value *LHS;
+  Value *RHS;
 
-  ReproducerEntry(CmpInst *Cond, bool IsNot) : Cond(Cond), IsNot(IsNot) {}
+  ReproducerEntry(ICmpInst::Predicate Pred, Value *LHS, Value *RHS)
+      : Pred(Pred), LHS(LHS), RHS(RHS) {}
 };
 } // namespace
 
 /// Helper function to generate a reproducer function for simplifying \p Cond.
 /// The reproducer function contains a series of @llvm.assume calls, one for
 /// each condition in \p Stack. For each condition, the operand instruction are
 /// cloned until we reach operands that have an entry in \p Value2Index. Those
 /// will then be added as function arguments. \p DT is used to order cloned
static void generateReproducer(CmpInst *Cond, Module *M,
   ValueToValueMapTy Old2New;
   SmallVector<Value *> Args;
   SmallPtrSet<Value *, 8> Seen;
   // Traverse Cond and its operands recursively until we reach a value that's in
   // Value2Index or not an instruction, or not a operation that
   // ConstraintElimination can decompose. Such values will be considered as
   // external inputs to the reproducer, they are collected and added as function
   // arguments later.
-  auto CollectArguments = [&](CmpInst *Cond) {
-    if (!Cond)
-      return;
-    auto &Value2Index =
-        Info.getValue2Index(CmpInst::isSigned(Cond->getPredicate()));
-    SmallVector<Value *, 4> WorkList;
-    WorkList.push_back(Cond);
+  auto CollectArguments = [&](ArrayRef<Value *> Ops, bool IsSigned) {
+    auto &Value2Index = Info.getValue2Index(IsSigned);
+    SmallVector<Value *, 4> WorkList(Ops);
     while (!WorkList.empty()) {
       Value *V = WorkList.pop_back_val();
       if (!Seen.insert(V).second)
         continue;
       if (Old2New.find(V) != Old2New.end())
         continue;
       if (isa<Constant>(V))
         continue;
 
       auto *I = dyn_cast<Instruction>(V);
       if (Value2Index.contains(V) || !I ||
           !isa<CmpInst, BinaryOperator, GEPOperator, CastInst>(V)) {
         Old2New[V] = V;
         Args.push_back(V);
         LLVM_DEBUG(dbgs() << "  found external input " << *V << "\n");
       } else {
         append_range(WorkList, I->operands());
       }
     }
   };
 
   for (auto &Entry : Stack)
-    CollectArguments(Entry.Cond);
-  CollectArguments(Cond);
+    if (Entry.Pred != ICmpInst::BAD_ICMP_PREDICATE)
+      CollectArguments({Entry.LHS, Entry.RHS}, ICmpInst::isSigned(Entry.Pred));
+  CollectArguments(Cond, ICmpInst::isSigned(Cond->getPredicate()));
 
   SmallVector<Type *> ParamTys;
   for (auto *P : Args)
     ParamTys.push_back(P->getType());
 
   FunctionType *FTy = FunctionType::get(Cond->getType(), ParamTys,
                                         /*isVarArg=*/false);
   Function *F = Function::Create(FTy, Function::ExternalLinkage,
static void generateReproducer(CmpInst *Cond, Module *M,
   };
 
   // Materialize the assumptions for the reproducer using the entries in Stack.
   // That is, first clone the operands of the condition recursively until we
   // reach an external input to the reproducer and add them to the reproducer
   // function. Then add an ICmp for the condition (with the inverse predicate if
   // the entry is negated) and an assert using the ICmp.
   for (auto &Entry : Stack) {
-    if (!Entry.Cond)
+    if (Entry.Pred == ICmpInst::BAD_ICMP_PREDICATE)
       continue;
 
-    LLVM_DEBUG(dbgs() << "  Materializing assumption " << *Entry.Cond << "\n");
-    CmpInst::Predicate Pred = Entry.Cond->getPredicate();
-    if (Entry.IsNot)
-      Pred = CmpInst::getInversePredicate(Pred);
-
-    CloneInstructions({Entry.Cond->getOperand(0), Entry.Cond->getOperand(1)},
-                      CmpInst::isSigned(Entry.Cond->getPredicate()));
+    LLVM_DEBUG(dbgs() << "  Materializing assumption " << Entry.Pred << ' ';
+               Entry.LHS->printAsOperand(dbgs()); dbgs() << ", ";
+               Entry.RHS->printAsOperand(dbgs()); dbgs() << "\n");
+    CloneInstructions({Entry.LHS, Entry.RHS}, CmpInst::isSigned(Entry.Pred));
 
-    auto *Cmp = Builder.CreateICmp(Pred, Entry.Cond->getOperand(0),
-                                   Entry.Cond->getOperand(1));
+    auto *Cmp = Builder.CreateICmp(Entry.Pred, Entry.LHS, Entry.RHS);
     Builder.CreateAssumption(Cmp);
   }
 
   // Finally, clone the condition to reproduce and remap instruction operands in
   // the reproducer using Old2New.
   CloneInstructions(Cond, CmpInst::isSigned(Cond->getPredicate()));
   Entry->getTerminator()->setOperand(0, Cond);
   remapInstructionsInBlocks({Entry}, Old2New);
if (match(Cmp, m_ICmp(Pred, m_Value(A), m_Value(B)))) {
       }
 
       // Use the inverse predicate if required.
       if (CB.Not)
         Pred = CmpInst::getInversePredicate(Pred);
 
       Info.addFact(Pred, A, B, CB.NumIn, CB.NumOut, DFSInStack);
       if (ReproducerModule && DFSInStack.size() > ReproducerCondStack.size())
-        ReproducerCondStack.emplace_back(cast<CmpInst>(Cmp), CB.Not);
+        ReproducerCondStack.emplace_back(Pred, A, B);
 
       Info.transferToOtherSystem(Pred, A, B, CB.NumIn, CB.NumOut, DFSInStack);
       if (ReproducerModule && DFSInStack.size() > ReproducerCondStack.size()) {
         // Add dummy entries to ReproducerCondStack to keep it in sync with
         // DFSInStack.
         for (unsigned I = 0,
                       E = (DFSInStack.size() - ReproducerCondStack.size());
              I < E; ++I) {
-          ReproducerCondStack.emplace_back(nullptr, false);
+          ReproducerCondStack.emplace_back(ICmpInst::BAD_ICMP_PREDICATE,
+                                           nullptr, nullptr);
         }
       }
     }
   }
 
   if (ReproducerModule && !ReproducerModule->functions().empty()) {
     std::string S;
     raw_string_ostream StringS(S);

llvm/test/Transforms/ConstraintElimination/reproducer-remarks-debug.ll

 ; RUN: opt -passes=constraint-elimination -constraint-elimination-dump-reproducers -pass-remarks=constraint-elimination -debug %s 2>&1 | FileCheck %s
 
 ; REQUIRES: asserts
 
 target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 
 ; CHECK:      Condition   %c.2 = icmp eq ptr %a, null implied by dominating constraints
 ; CHECK-NEXT: %a + -1 * null <= 0
 ; CHECK-NEXT: Creating reproducer for   %c.2 = icmp eq ptr %a, null
 ; CHECK-NEXT:   found external input ptr %a
-; CHECK-NEXT:   Materializing assumption   %c.1 = icmp eq ptr %a, null
+; CHECK-NEXT:   Materializing assumption eq ptr %a, ptr null
 
 define i1 @test_ptr_null_constant(ptr %a) {
 ; CHECK-LABEL: define i1 @"{{.+}}test_ptr_null_constantrepro"(ptr %a) {
 ; CHECK-NEXT: entry:
 ; CHECK-NEXT:   %0 = icmp eq ptr %a, null
 ; CHECK-NEXT:   call void @llvm.assume(i1 %0)
 ; CHECK-NEXT:   %c.2 = icmp eq ptr %a, null
 ; CHECK-NEXT:   ret i1 %c.2