[PATCH] D21271: Fix `InstCombine` to not widen metadata on store-to-load forwarding

[Yichao Yu via llvm-commits]

yuyichao created this revision.
yuyichao added reviewers: dotdash, majnemer, loladiro, carnaval, vtjnash.
yuyichao added a subscriber: llvm-commits.

The original check for load CSE or store-to-load forwarding is wrong
when the forwarded stored value happened to be a load.

The current logic was added in r241886. It looks safe but I'm wondering what kind of code pattern it used to mis-optimize. If there are two loads from the same address marked with either metadata, shouldn't it be valid to keep either of them? (After all, either of them had better be valid to start with).

Ref https://github.com/JuliaLang/julia/issues/16894


http://reviews.llvm.org/D21271

Files:
  include/llvm/Analysis/Loads.h
  lib/Analysis/Loads.cpp
  lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
  test/Transforms/InstCombine/tbaa-store-to-load.ll

Index: test/Transforms/InstCombine/tbaa-store-to-load.ll
===================================================================
--- /dev/null
+++ test/Transforms/InstCombine/tbaa-store-to-load.ll
@@ -0,0 +1,17 @@
+; RUN: opt -S -instcombine < %s 2>&1 | FileCheck %s
+
+define i64 @f(i64* %p1, i64* %p2) {
+top:
+  ; check that the tbaa is preserved
+  ; CHECK-LABEL: @f(
+  ; CHECK: %v1 = load i64, i64* %p1, align 8, !tbaa !0
+  ; CHECK: store i64 %v1, i64* %p2, align 8
+  ; CHECK: ret i64 %v1
+  %v1 = load i64, i64* %p1, align 8, !tbaa !0
+  store i64 %v1, i64* %p2, align 8
+  %v2 = load i64, i64* %p2, align 8
+  ret i64 %v2
+}
+
+!0 = !{!1, !1, i64 0}
+!1 = !{!"load_tbaa"}
Index: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
===================================================================
--- lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -819,10 +819,12 @@
   // separated by a few arithmetic operations.
   BasicBlock::iterator BBI(LI);
   AAMDNodes AATags;
+  bool IsLoadCSE = false;
   if (Value *AvailableVal =
       FindAvailableLoadedValue(&LI, LI.getParent(), BBI,
-                               DefMaxInstsToScan, AA, &AATags)) {
-    if (LoadInst *NLI = dyn_cast<LoadInst>(AvailableVal)) {
+                               DefMaxInstsToScan, AA, &AATags, &IsLoadCSE)) {
+    if (IsLoadCSE) {
+      LoadInst *NLI = static_cast<LoadInst*>(AvailableVal);
       unsigned KnownIDs[] = {
           LLVMContext::MD_tbaa,            LLVMContext::MD_alias_scope,
           LLVMContext::MD_noalias,         LLVMContext::MD_range,
Index: lib/Analysis/Loads.cpp
===================================================================
--- lib/Analysis/Loads.cpp
+++ lib/Analysis/Loads.cpp
@@ -322,7 +322,8 @@
 Value *llvm::FindAvailableLoadedValue(LoadInst *Load, BasicBlock *ScanBB,
                                       BasicBlock::iterator &ScanFrom,
                                       unsigned MaxInstsToScan,
-                                      AliasAnalysis *AA, AAMDNodes *AATags) {
+                                      AliasAnalysis *AA, AAMDNodes *AATags,
+                                      bool *IsLoadCSE) {
   if (MaxInstsToScan == 0)
     MaxInstsToScan = ~0U;
 
@@ -374,6 +375,8 @@
 
         if (AATags)
           LI->getAAMetadata(*AATags);
+        if (IsLoadCSE)
+            *IsLoadCSE = true;
         return LI;
       }
 
Index: include/llvm/Analysis/Loads.h
===================================================================
--- include/llvm/Analysis/Loads.h
+++ include/llvm/Analysis/Loads.h
@@ -82,7 +82,8 @@
                                 BasicBlock::iterator &ScanFrom,
                                 unsigned MaxInstsToScan = DefMaxInstsToScan,
                                 AliasAnalysis *AA = nullptr,
-                                AAMDNodes *AATags = nullptr);
+                                AAMDNodes *AATags = nullptr,
+                                bool *IsLoadCSE = nullptr);
 
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D21271.60480.patch
Type: text/x-patch
Size: 3045 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160612/c3f5b4b6/attachment.bin>