[JumpThreading][GVN] Copy metadata when inserting preload into preds

[dtcxzyw]

This patch propagates metadata to the new load instruction when inserting a preload into predecessors. It also changes GVN to use the same logic.

See also #134093 (comment) for the motivation.

[llvmbot]

@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

This patch propagates metadata to the new load instruction when inserting a preload into predecessors. It also changes GVN to use the common helper copyMetadataForLoad.
See also #134093 (comment) for the motivation.

---

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

4 Files Affected:

• (modified) llvm/lib/Transforms/Scalar/GVN.cpp (+1-20)
• (modified) llvm/lib/Transforms/Scalar/JumpThreading.cpp (+1-2)
• (modified) llvm/lib/Transforms/Utils/Local.cpp (+1)
• (modified) llvm/test/Transforms/JumpThreading/pre-load.ll (+51-1)

diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index 6233e8e2ee681..31bdbc95fc277 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -1516,26 +1516,7 @@ void GVNPass::eliminatePartiallyRedundantLoad(
         MSSAU->insertUse(cast<MemoryUse>(NewAccess), /*RenameUses=*/true);
     }
 
-    // Transfer the old load's AA tags to the new load.
-    AAMDNodes Tags = Load->getAAMetadata();
-    if (Tags)
-      NewLoad->setAAMetadata(Tags);
-
-    if (auto *MD = Load->getMetadata(LLVMContext::MD_invariant_load))
-      NewLoad->setMetadata(LLVMContext::MD_invariant_load, MD);
-    if (auto *InvGroupMD = Load->getMetadata(LLVMContext::MD_invariant_group))
-      NewLoad->setMetadata(LLVMContext::MD_invariant_group, InvGroupMD);
-    if (auto *RangeMD = Load->getMetadata(LLVMContext::MD_range))
-      NewLoad->setMetadata(LLVMContext::MD_range, RangeMD);
-    if (auto *AccessMD = Load->getMetadata(LLVMContext::MD_access_group))
-      if (LI->getLoopFor(Load->getParent()) == LI->getLoopFor(UnavailableBlock))
-        NewLoad->setMetadata(LLVMContext::MD_access_group, AccessMD);
-
-    // We do not propagate the old load's debug location, because the new
-    // load now lives in a different BB, and we want to avoid a jumpy line
-    // table.
-    // FIXME: How do we retain source locations without causing poor debugging
-    // behavior?
+    copyMetadataForLoad(*NewLoad, *Load);
 
     // Add the newly created load.
     ValuesPerBlock.push_back(
diff --git a/llvm/lib/Transforms/Scalar/JumpThreading.cpp b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
index ba598d8415b18..8d790648c2ef9 100644
--- a/llvm/lib/Transforms/Scalar/JumpThreading.cpp
+++ b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
@@ -1407,8 +1407,7 @@ bool JumpThreadingPass::simplifyPartiallyRedundantLoad(LoadInst *LoadI) {
         LoadI->getOrdering(), LoadI->getSyncScopeID(),
         UnavailablePred->getTerminator()->getIterator());
     NewVal->setDebugLoc(LoadI->getDebugLoc());
-    if (AATags)
-      NewVal->setAAMetadata(AATags);
+    copyMetadataForLoad(*NewVal, *LoadI);
 
     AvailablePreds.emplace_back(UnavailablePred, NewVal);
   }
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index 2f3ea2266e07f..fab45b1dfb088 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -3468,6 +3468,7 @@ void llvm::copyMetadataForLoad(LoadInst &Dest, const LoadInst &Source) {
     case LLVMContext::MD_fpmath:
     case LLVMContext::MD_tbaa_struct:
     case LLVMContext::MD_invariant_load:
+    case LLVMContext::MD_invariant_group:
     case LLVMContext::MD_alias_scope:
     case LLVMContext::MD_noalias:
     case LLVMContext::MD_nontemporal:
diff --git a/llvm/test/Transforms/JumpThreading/pre-load.ll b/llvm/test/Transforms/JumpThreading/pre-load.ll
index d9a2dc20a4189..eef7735acf448 100644
--- a/llvm/test/Transforms/JumpThreading/pre-load.ll
+++ b/llvm/test/Transforms/JumpThreading/pre-load.ll
@@ -1,4 +1,4 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals
 ; RUN: opt -passes=jump-threading -S < %s | FileCheck %s
 
 @x = global i32 0
@@ -7,6 +7,10 @@
 declare void @f()
 declare void @g()
 
+;.
+; CHECK: @x = global i32 0
+; CHECK: @y = global i32 0
+;.
 define i32 @pre(i1 %cond, i32 %n) {
 ; CHECK-LABEL: @pre(
 ; CHECK-NEXT:    br i1 [[COND:%.*]], label [[C_THREAD:%.*]], label [[C:%.*]]
@@ -82,3 +86,49 @@ NO:
   call void @g()
   ret i32 1
 }
+
+define i32 @pre_metadata(i1 %cond) {
+; CHECK-LABEL: @pre_metadata(
+; CHECK-NEXT:    br i1 [[COND:%.*]], label [[C_THREAD:%.*]], label [[C:%.*]]
+; CHECK:       C.thread:
+; CHECK-NEXT:    store i32 0, ptr @x, align 4
+; CHECK-NEXT:    br label [[YES:%.*]]
+; CHECK:       C:
+; CHECK-NEXT:    [[A_PR:%.*]] = load i32, ptr @y, align 4, !range [[RNG0:![0-9]+]], !noundef [[META1:![0-9]+]]
+; CHECK-NEXT:    [[COND2:%.*]] = icmp eq i32 [[A_PR]], 0
+; CHECK-NEXT:    br i1 [[COND2]], label [[YES]], label [[NO:%.*]]
+; CHECK:       YES:
+; CHECK-NEXT:    [[A4:%.*]] = phi i32 [ 0, [[C_THREAD]] ], [ [[A_PR]], [[C]] ]
+; CHECK-NEXT:    call void @f()
+; CHECK-NEXT:    ret i32 [[A4]]
+; CHECK:       NO:
+; CHECK-NEXT:    call void @g()
+; CHECK-NEXT:    ret i32 1
+;
+  br i1 %cond, label %A, label %B
+
+A:
+  store i32 0, ptr @x, align 4
+  br label %C
+
+B:
+  br label %C
+
+C:
+  %ptr = phi ptr [@x, %A], [@y, %B]
+  %a = load i32, ptr %ptr, align 4, !range !{i32 0, i32 2}, !noundef !{}
+  %cond2 = icmp eq i32 %a, 0
+  br i1 %cond2, label %YES, label %NO
+
+YES:
+  call void @f()
+  ret i32 %a
+
+NO:
+  call void @g()
+  ret i32 1
+}
+;.
+; CHECK: [[RNG0]] = !{i32 0, i32 2}
+; CHECK: [[META1]] = !{}
+;.

[dtcxzyw]

dtcxzyw/llvm-opt-benchmark#2295 We still need to salvage the UB-implying metadata in GVN (especially for !tbaa).

[dtcxzyw]

I am not sure if it holds for GVN.

[dtcxzyw]

Compile-time impact is high: https://llvm-compile-time-tracker.com/compare.php?from=89b09c80ebc1a198bf2eab81b5fcb9ba71a0f066&to=544f796aaf9c89e474928a3cca24c95647c09eb1&stat=instructions:u