[llvm] [GVN] Skip instructions without value numbers in performScalarPRE (PR #219851)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: Nikhil Ludder (badnikhil)

<details>
<summary>Changes</summary>

GVN may insert coercion instructions (MaterializeAdjustedValue) without assigning them value numbers. If one becomes a GEP index of a PRE'd load in the same processBlock sweep, the GEP-index scalar PRE in processNonLocalLoad reaches it before the next re-numbering iteration and the verifying VN.lookup() asserts "Value not numbered?".

Bail out instead: a just-inserted instruction has no leaders, so PRE cannot transform it this iteration; the next GVN iteration numbers it and handles it normally.

Fixes #<!-- -->216265.

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


2 Files Affected:

- (modified) llvm/lib/Transforms/Scalar/GVN.cpp (+7-1) 
- (added) llvm/test/Transforms/GVN/pre-coerced-value-not-numbered.ll (+41) 


``````````diff
diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index bd3fceb1ff3d2..64e3aaadc71cc 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -3643,7 +3643,13 @@ bool GVNPass::performScalarPRE(Instruction *CurInst) {
       return false;
   }
 
-  uint32_t ValNo = VN.lookup(CurInst);
+  // Instructions inserted by GVN itself in the current iteration, e.g. when
+  // coercing an available load value to the load type, have not been assigned
+  // a value number yet. They have no leaders either, so PRE cannot do anything
+  // with them until they are numbered on the next iteration; skip them.
+  uint32_t ValNo = VN.lookup(CurInst, /*Verify=*/false);
+  if (!ValNo)
+    return false;
 
   // Look for the predecessors for PRE opportunities.  We're
   // only trying to solve the basic diamond case, where
diff --git a/llvm/test/Transforms/GVN/pre-coerced-value-not-numbered.ll b/llvm/test/Transforms/GVN/pre-coerced-value-not-numbered.ll
new file mode 100644
index 0000000000000..1ca08c5709142
--- /dev/null
+++ b/llvm/test/Transforms/GVN/pre-coerced-value-not-numbered.ll
@@ -0,0 +1,41 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6
+; RUN: opt -passes=gvn -S < %s | FileCheck %s
+
+; Test for #216265. When GVN forwards the vector store to the i64 load, it
+; inserts a coercion bitcast that has no value number yet. Processing the
+; PRE'd load of %gep afterwards runs scalar PRE on the GEP index (now the
+; bitcast) and used to assert with "Value not numbered?".
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+
+ at g14 = external global i64
+ at g23 = external global <2 x i32>
+
+define void @f2() {
+; CHECK-LABEL: define void @f2() {
+; CHECK-NEXT:  [[ENTRY:.*]]:
+; CHECK-NEXT:    [[ARR:%.*]] = alloca [9 x i8], align 1
+; CHECK-NEXT:    [[V:%.*]] = load <2 x i32>, ptr @g14, align 8
+; CHECK-NEXT:    br label %[[LOOP:.*]]
+; CHECK:       [[LOOP]]:
+; CHECK-NEXT:    [[IV:%.*]] = phi i64 [ 0, %[[ENTRY]] ], [ [[TMP0:%.*]], %[[LOOP]] ]
+; CHECK-NEXT:    [[GEP:%.*]] = getelementptr inbounds i8, ptr [[ARR]], i64 [[IV]]
+; CHECK-NEXT:    store <2 x i32> [[V]], ptr @g23, align 8
+; CHECK-NEXT:    [[TMP0]] = bitcast <2 x i32> [[V]] to i64
+; CHECK-NEXT:    [[GEP_PHI_TRANS_INSERT:%.*]] = getelementptr inbounds i8, ptr [[ARR]], i64 [[TMP0]]
+; CHECK-NEXT:    br label %[[LOOP]]
+;
+entry:
+  %arr = alloca [9 x i8], align 1
+  %v = load <2 x i32>, ptr @g14, align 8
+  br label %loop
+
+loop:
+  %iv = phi i64 [ 0, %entry ], [ %conv, %loop ]
+  %gep = getelementptr inbounds i8, ptr %arr, i64 %iv
+  %load1 = load i8, ptr %gep, align 1
+  %t = trunc nuw i8 %load1 to i1
+  store <2 x i32> %v, ptr @g23, align 8
+  %conv = load i64, ptr @g23, align 8
+  br label %loop
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/219851