[NewGVN] Set parent to the temporal instructions that are generated during phi-of-ops optimization

[kmitropoulou]

• Test for future commit in NewGVN
• [NewGVN] Set parent to the temporal instructions that are generated during phi-of-ops optimization

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Changes

- Test for future commit in NewGVN - [NewGVN] Set parent to the temporal instructions that are generated during phi-of-ops optimization

--
Full diff: https://github.com/llvm/llvm-project/pull/66314.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/Scalar/NewGVN.cpp (+9-2)
• (added) llvm/test/Transforms/NewGVN/assume_dominating_icmp.ll (+64)

diff --git a/llvm/lib/Transforms/Scalar/NewGVN.cpp b/llvm/lib/Transforms/Scalar/NewGVN.cpp
index 81cc08168577581..b258a6b429f2267 100644
--- a/llvm/lib/Transforms/Scalar/NewGVN.cpp
+++ b/llvm/lib/Transforms/Scalar/NewGVN.cpp
@@ -538,6 +538,8 @@ class NewGVN {
   // created that they are known equivalent to.
   DenseMap<const Value *, PHINode *> RealToTemp;
 
+  Value *PhiOfOpsCandidate;
+
   // In order to know when we should re-process instructions that have
   // phi-of-ops, we track the set of expressions that they needed as
   // leaders. When we discover new leaders for those expressions, we process the
@@ -1904,7 +1906,10 @@ NewGVN::ExprResult NewGVN::performSymbolicCmpEvaluation(Instruction *I) const {
       LastPredInfo = PI;
       // In phi of ops cases, we may have predicate info that we are evaluating
       // in a different context.
-      if (!DT->dominates(PBranch->To, getBlockForValue(I)))
+      BasicBlock *From = PBranch->To;
+      BasicBlock *To =
+          !PhiOfOpsCandidate ? I->getParent() : TempToBlock.lookup(I);
+      if (!DT->dominates(From, To))
         continue;
       // TODO: Along the false edge, we may know more things too, like
       // icmp of
@@ -2666,6 +2671,7 @@ Value *NewGVN::findLeaderForInst(Instruction *TransInst,
   TempToBlock.insert({TransInst, PredBB});
   InstrDFS.insert({TransInst, IDFSNum});
 
+  PhiOfOpsCandidate = TransInst;
   auto Res = performSymbolicEvaluation(TransInst, Visited);
   const Expression *E = Res.Expr;
   addAdditionalUsers(Res, OrigInst);
@@ -2765,6 +2771,7 @@ NewGVN::makePossiblePHIOfOps(Instruction *I,
       // Clone the instruction, create an expression from it that is
       // translated back into the predecessor, and see if we have a leader.
       Instruction *ValueOp = I->clone();
+      ValueOp->insertBefore(I);
       if (MemAccess)
         TempToMemory.insert({ValueOp, MemAccess});
       bool SafeForPHIOfOps = true;
@@ -2794,7 +2801,7 @@ NewGVN::makePossiblePHIOfOps(Instruction *I,
       FoundVal = !SafeForPHIOfOps ? nullptr
                                   : findLeaderForInst(ValueOp, Visited,
                                                       MemAccess, I, PredBB);
-      ValueOp->deleteValue();
+      ValueOp->eraseFromParent();
       if (!FoundVal) {
         // We failed to find a leader for the current ValueOp, but this might
         // change in case of the translated operands change.
diff --git a/llvm/test/Transforms/NewGVN/assume_dominating_icmp.ll b/llvm/test/Transforms/NewGVN/assume_dominating_icmp.ll
new file mode 100644
index 000000000000000..1bf9f65535c3c58
--- /dev/null
+++ b/llvm/test/Transforms/NewGVN/assume_dominating_icmp.ll
@@ -0,0 +1,64 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 3
+; RUN: opt -passes=newgvn -S < %s | FileCheck %s
+
+@c = global i32 0, align 4
+
+; Function Attrs: nounwind optsize uwtable
+define dso_local i32 @main(i1 %cond, i32 %0, i32 %1) {
+; CHECK-LABEL: define dso_local i32 @main(
+; CHECK-SAME: i1 [[COND:%.*]], i32 [[TMP0:%.*]], i32 [[TMP1:%.*]]) {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br i1 [[COND]], label [[IF_THEN:%.*]], label [[IF_END6:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    [[XOR:%.*]] = xor i32 [[TMP0]], 1
+; CHECK-NEXT:    [[TOBOOL1_NOT:%.*]] = icmp eq i32 [[TMP1]], 0
+; CHECK-NEXT:    br i1 [[TOBOOL1_NOT]], label [[IF_END6]], label [[IF_THEN2:%.*]]
+; CHECK:       if.then2:
+; CHECK-NEXT:    [[TOBOOL3_NOT:%.*]] = icmp ne i32 [[XOR]], 0
+; CHECK-NEXT:    tail call void @llvm.assume(i1 [[TOBOOL3_NOT]])
+; CHECK-NEXT:    br label [[IF_END6]]
+; CHECK:       if.end6:
+; CHECK-NEXT:    [[F_0:%.*]] = phi i32 [ undef, [[ENTRY:%.*]] ], [ [[XOR]], [[IF_THEN]] ], [ [[XOR]], [[IF_THEN2]] ]
+; CHECK-NEXT:    [[NOT:%.*]] = xor i32 [[F_0]], -1
+; CHECK-NEXT:    [[TMP2:%.*]] = load i32, ptr @c, align 4
+; CHECK-NEXT:    [[OR:%.*]] = or i32 [[TMP2]], [[NOT]]
+; CHECK-NEXT:    [[TOBOOL7_NOT:%.*]] = icmp eq i32 [[OR]], 0
+; CHECK-NEXT:    [[TOBOOL9_NOT:%.*]] = icmp eq i32 [[F_0]], 0
+; CHECK-NEXT:    [[OR_COND:%.*]] = or i1 [[TOBOOL7_NOT]], [[TOBOOL9_NOT]]
+; CHECK-NEXT:    br i1 [[OR_COND]], label [[IF_END10:%.*]], label [[WHILE_COND_PREHEADER:%.*]]
+; CHECK:       while.cond.preheader:
+; CHECK-NEXT:    ret i32 1
+; CHECK:       if.end10:
+; CHECK-NEXT:    ret i32 0
+;
+entry:
+  br i1 %cond, label %if.then, label %if.end6
+
+if.then:                                          ; preds = %entry
+  %xor = xor i32 %0, 1
+  %tobool1.not = icmp eq i32 %1, 0
+  br i1 %tobool1.not, label %if.end6, label %if.then2
+
+if.then2:                                         ; preds = %if.then
+  %tobool3.not = icmp ne i32 %xor, 0
+  tail call void @llvm.assume(i1 %tobool3.not)
+  br label %if.end6
+
+if.end6:                                          ; preds = %if.then2, %if.then, %entry
+  %f.0 = phi i32 [ undef, %entry ], [ %xor, %if.then ], [ %xor, %if.then2 ]
+  %not = xor i32 %f.0, -1
+  %2 = load i32, ptr @c, align 4
+  %or = or i32 %2, %not
+  %tobool7.not = icmp eq i32 %or, 0
+  %tobool9.not = icmp eq i32 %f.0, 0
+  %or.cond = or i1 %tobool7.not, %tobool9.not
+  br i1 %or.cond, label %if.end10, label %while.cond.preheader
+
+while.cond.preheader:                             ; preds = %if.end6
+  ret i32 1
+
+if.end10:                                         ; preds = %if.end6
+  ret i32 0
+}
+
+declare void @llvm.assume(i1 noundef)

[kmitropoulou]

This patch depends on #66313

[nikic]

LGTM