[llvm] 084ca63 - [EarlyCSE] Only combine metadata for load CSE

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2023-05-02T12:51:56+02:00
New Revision: 084ca632ac81437e5517ae5933b22d2b8e9a2bcd

URL: https://github.com/llvm/llvm-project/commit/084ca632ac81437e5517ae5933b22d2b8e9a2bcd
DIFF: https://github.com/llvm/llvm-project/commit/084ca632ac81437e5517ae5933b22d2b8e9a2bcd.diff

LOG: [EarlyCSE] Only combine metadata for load CSE

There is no need to combine metadata if we're performing store to
load forwarding. In that case we would end up combining metadata
on an unrelated load instruction.

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/EarlyCSE.cpp
    llvm/test/Transforms/EarlyCSE/flags.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/EarlyCSE.cpp b/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
index 09fecc137bad..3ac315a4f9fa 100644
--- a/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
+++ b/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
@@ -594,12 +594,13 @@ class EarlyCSE {
     unsigned Generation = 0;
     int MatchingId = -1;
     bool IsAtomic = false;
+    bool IsLoad = false;
 
     LoadValue() = default;
     LoadValue(Instruction *Inst, unsigned Generation, unsigned MatchingId,
-              bool IsAtomic)
+              bool IsAtomic, bool IsLoad)
         : DefInst(Inst), Generation(Generation), MatchingId(MatchingId),
-          IsAtomic(IsAtomic) {}
+          IsAtomic(IsAtomic), IsLoad(IsLoad) {}
   };
 
   using LoadMapAllocator =
@@ -1492,8 +1493,9 @@ bool EarlyCSE::processNode(DomTreeNode *Node) {
           LLVM_DEBUG(dbgs() << "Skipping due to debug counter\n");
           continue;
         }
-        if (auto *I = dyn_cast<Instruction>(Op))
-          combineMetadataForCSE(I, &Inst, false);
+        if (InVal.IsLoad)
+          if (auto *I = dyn_cast<Instruction>(Op))
+            combineMetadataForCSE(I, &Inst, false);
         if (!Inst.use_empty())
           Inst.replaceAllUsesWith(Op);
         salvageKnowledge(&Inst, &AC);
@@ -1508,7 +1510,8 @@ bool EarlyCSE::processNode(DomTreeNode *Node) {
       AvailableLoads.insert(MemInst.getPointerOperand(),
                             LoadValue(&Inst, CurrentGeneration,
                                       MemInst.getMatchingId(),
-                                      MemInst.isAtomic()));
+                                      MemInst.isAtomic(),
+                                      MemInst.isLoad()));
       LastStore = nullptr;
       continue;
     }
@@ -1632,7 +1635,8 @@ bool EarlyCSE::processNode(DomTreeNode *Node) {
         AvailableLoads.insert(MemInst.getPointerOperand(),
                               LoadValue(&Inst, CurrentGeneration,
                                         MemInst.getMatchingId(),
-                                        MemInst.isAtomic()));
+                                        MemInst.isAtomic(),
+                                        MemInst.isLoad()));
 
         // Remember that this was the last unordered store we saw for DSE. We
         // don't yet handle DSE on ordered or volatile stores since we don't

diff  --git a/llvm/test/Transforms/EarlyCSE/flags.ll b/llvm/test/Transforms/EarlyCSE/flags.ll
index 371fc26dac80..78282b081c29 100644
--- a/llvm/test/Transforms/EarlyCSE/flags.ll
+++ b/llvm/test/Transforms/EarlyCSE/flags.ll
@@ -93,7 +93,7 @@ define void @load_first_nonnull_noundef(ptr %p) {
 
 define ptr @store_to_load_forward(ptr %p, ptr %p2) {
 ; CHECK-LABEL: @store_to_load_forward(
-; CHECK-NEXT:    [[P3:%.*]] = load ptr, ptr [[P:%.*]], align 8
+; CHECK-NEXT:    [[P3:%.*]] = load ptr, ptr [[P:%.*]], align 8, !nonnull !0
 ; CHECK-NEXT:    store ptr [[P3]], ptr [[P2:%.*]], align 8
 ; CHECK-NEXT:    ret ptr [[P3]]
 ;