[llvm] 20e9b31 - [GVN] Fix metadata combining for non-local loads

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2023-04-20T12:32:10+02:00
New Revision: 20e9b31f88149a1d5ef78c0be50051e345098e41

URL: https://github.com/llvm/llvm-project/commit/20e9b31f88149a1d5ef78c0be50051e345098e41
DIFF: https://github.com/llvm/llvm-project/commit/20e9b31f88149a1d5ef78c0be50051e345098e41.diff

LOG: [GVN] Fix metadata combining for non-local loads

Make MaterializeAdjustedValue() responsible for adjusting load
metadata in all cases, so it also covers the non-local case.

In conjunction with that, we no longer need to call
patchReplacementInstruction() for the local case, which would
unnecessarily drop metadata if the replacement value just happened
to be a load (without actual load CSE).

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/GVN.cpp
    llvm/test/Transforms/GVN/metadata.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index 291a426f20cfa..098ea89d170fb 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -997,6 +997,7 @@ Value *AvailableValue::MaterializeAdjustedValue(LoadInst *Load,
     LoadInst *CoercedLoad = getCoercedLoadValue();
     if (CoercedLoad->getType() == LoadTy && Offset == 0) {
       Res = CoercedLoad;
+      combineMetadataForCSE(CoercedLoad, Load, false);
     } else {
       Res = getValueForLoad(CoercedLoad, Offset, LoadTy, InsertPt, DL);
       // We are adding a new user for this load, for which the original
@@ -1382,6 +1383,7 @@ void GVNPass::eliminatePartiallyRedundantLoad(
 
   // Perform PHI construction.
   Value *V = ConstructSSAForLoadSet(Load, ValuesPerBlock, *this);
+  // ConstructSSAForLoadSet is responsible for combining metadata.
   Load->replaceAllUsesWith(V);
   if (isa<PHINode>(V))
     V->takeName(Load);
@@ -1781,6 +1783,7 @@ bool GVNPass::processNonLocalLoad(LoadInst *Load) {
 
     // Perform PHI construction.
     Value *V = ConstructSSAForLoadSet(Load, ValuesPerBlock, *this);
+    // ConstructSSAForLoadSet is responsible for combining metadata.
     Load->replaceAllUsesWith(V);
 
     if (isa<PHINode>(V))
@@ -2054,8 +2057,8 @@ bool GVNPass::processLoad(LoadInst *L) {
 
   Value *AvailableValue = AV->MaterializeAdjustedValue(L, L, *this);
 
-  // Replace the load!
-  patchAndReplaceAllUsesWith(L, AvailableValue);
+  // MaterializeAdjustedValue is responsible for combining metadata.
+  L->replaceAllUsesWith(AvailableValue);
   markInstructionForDeletion(L);
   if (MSSAU)
     MSSAU->removeMemoryAccess(L);

diff  --git a/llvm/test/Transforms/GVN/metadata.ll b/llvm/test/Transforms/GVN/metadata.ll
index 8034e5dc4fb83..b9e7c00a6c757 100644
--- a/llvm/test/Transforms/GVN/metadata.ll
+++ b/llvm/test/Transforms/GVN/metadata.ll
@@ -287,7 +287,7 @@ define void @load_i64_range_to_i32_range(ptr %p) {
 define i64 @load_is_stored(ptr %p, ptr %p2) {
 ; CHECK-LABEL: define i64 @load_is_stored
 ; CHECK-SAME: (ptr [[P:%.*]], ptr [[P2:%.*]]) {
-; CHECK-NEXT:    [[V1:%.*]] = load i64, ptr [[P]], align 4
+; CHECK-NEXT:    [[V1:%.*]] = load i64, ptr [[P]], align 4, !range [[RNG8:![0-9]+]]
 ; CHECK-NEXT:    store i64 [[V1]], ptr [[P2]], align 4
 ; CHECK-NEXT:    ret i64 [[V1]]
 ;
@@ -300,7 +300,7 @@ define i64 @load_is_stored(ptr %p, ptr %p2) {
 define void @non_local_dominating(i1 %c, ptr %p) {
 ; CHECK-LABEL: define void @non_local_dominating
 ; CHECK-SAME: (i1 [[C:%.*]], ptr [[P:%.*]]) {
-; CHECK-NEXT:    [[V1:%.*]] = load i64, ptr [[P]], align 4, !range [[RNG8:![0-9]+]]
+; CHECK-NEXT:    [[V1:%.*]] = load i64, ptr [[P]], align 4, !range [[RNG9:![0-9]+]]
 ; CHECK-NEXT:    br i1 [[C]], label [[IF:%.*]], label [[JOIN:%.*]]
 ; CHECK:       if:
 ; CHECK-NEXT:    br label [[JOIN]]
@@ -327,11 +327,11 @@ define void @non_local_non_dominating(i1 %c, ptr %p) {
 ; CHECK-SAME: (i1 [[C:%.*]], ptr [[P:%.*]]) {
 ; CHECK-NEXT:    br i1 [[C]], label [[IF:%.*]], label [[ELSE:%.*]]
 ; CHECK:       if:
-; CHECK-NEXT:    [[V1:%.*]] = load i64, ptr [[P]], align 4, !range [[RNG8]]
+; CHECK-NEXT:    [[V1:%.*]] = load i64, ptr [[P]], align 4, !range [[RNG9]]
 ; CHECK-NEXT:    call void @use.i64(i64 [[V1]])
 ; CHECK-NEXT:    br label [[JOIN:%.*]]
 ; CHECK:       else:
-; CHECK-NEXT:    [[V2:%.*]] = load i64, ptr [[P]], align 4, !range [[RNG9:![0-9]+]]
+; CHECK-NEXT:    [[V2:%.*]] = load i64, ptr [[P]], align 4, !range [[RNG10:![0-9]+]]
 ; CHECK-NEXT:    call void @use.i64(i64 [[V2]])
 ; CHECK-NEXT:    br label [[JOIN]]
 ; CHECK:       join:
@@ -389,10 +389,10 @@ define void @non_local_pre(i1 %c, ptr %p) {
 ; CHECK-SAME: (i1 [[C:%.*]], ptr [[P:%.*]]) {
 ; CHECK-NEXT:    br i1 [[C]], label [[IF:%.*]], label [[DOTJOIN_CRIT_EDGE:%.*]]
 ; CHECK:       .join_crit_edge:
-; CHECK-NEXT:    [[V2_PRE:%.*]] = load i64, ptr [[P]], align 4, !range [[RNG10:![0-9]+]]
+; CHECK-NEXT:    [[V2_PRE:%.*]] = load i64, ptr [[P]], align 4, !range [[RNG11:![0-9]+]]
 ; CHECK-NEXT:    br label [[JOIN:%.*]]
 ; CHECK:       if:
-; CHECK-NEXT:    [[V1:%.*]] = load i64, ptr [[P]], align 4, !range [[RNG8]]
+; CHECK-NEXT:    [[V1:%.*]] = load i64, ptr [[P]], align 4, !range [[RNG9]]
 ; CHECK-NEXT:    call void @use.i64(i64 [[V1]])
 ; CHECK-NEXT:    br label [[JOIN]]
 ; CHECK:       join:
@@ -437,6 +437,7 @@ join:
 ; CHECK: [[META6:![0-9]+]] = !{}
 ; CHECK: [[META7:![0-9]+]] = !{i64 10}
 ; CHECK: [[RNG8]] = !{i64 0, i64 10}
-; CHECK: [[RNG9]] = !{i64 10, i64 20}
-; CHECK: [[RNG10]] = !{i64 20, i64 30}
+; CHECK: [[RNG9]] = !{i64 0, i64 10, i64 20, i64 30}
+; CHECK: [[RNG10]] = !{i64 10, i64 30}
+; CHECK: [[RNG11]] = !{i64 20, i64 30}
 ;.