Skip to content

Commit

Permalink
Add LLVM patches to fix InstCombine loosing tbaa metadata
Browse files Browse the repository at this point in the history 
Fixes #16894
  • Loading branch information
@yuyichao
yuyichao committed Jun 16, 2016
1 parent e261293 commit b03c3e3
Show file tree
Hide file tree
Showing 3 changed files with 197 additions and 0 deletions.
2 changes: 2 additions & 0 deletions deps/llvm.mk
View file
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,7 @@ $(eval $(call LLVM_PATCH,llvm-3.7.1_symlinks))
$(eval $(call LLVM_PATCH,llvm-3.8.0_bindir))
$(eval $(call LLVM_PATCH,llvm-D14260))
$(eval $(call LLVM_PATCH,llvm-nodllalias))
$(eval $(call LLVM_PATCH,llvm-D21271-instcombine-tbaa-3.7))
else ifeq ($(LLVM_VER),3.8.0)
$(eval $(call LLVM_PATCH,llvm-3.7.1_3))
$(eval $(call LLVM_PATCH,llvm-D14260))
Expand All @@ -445,6 +446,7 @@ $(eval $(call LLVM_PATCH,llvm-D17712))
$(eval $(call LLVM_PATCH,llvm-PR26180))
$(eval $(call LLVM_PATCH,llvm-PR27046))
$(eval $(call LLVM_PATCH,llvm-3.8.0_ppc64_SUBFC8))
$(eval $(call LLVM_PATCH,llvm-D21271-instcombine-tbaa-3.8))
endif # LLVM_VER

ifeq ($(LLVM_VER),3.7.1)
Expand Down
97 changes: 97 additions & 0 deletions deps/patches/llvm-D21271-instcombine-tbaa-3.7.patch
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
From a3c3e1bd22fef6de6d31b3a80d773d1176b4ca8c Mon Sep 17 00:00:00 2001
From: Yichao Yu <[email protected]>
Date: Sun, 12 Jun 2016 17:14:38 -0400
Subject: [PATCH] Fix `InstCombine` to not widen metadata on store-to-load
forwarding.

The original check for load CSE or store-to-load forwarding is wrong
when the forwarded stored value happened to be a load.
---
include/llvm/Analysis/Loads.h | 3 ++-
lib/Analysis/Loads.cpp | 5 ++++-
.../InstCombine/InstCombineLoadStoreAlloca.cpp | 6 ++++--
test/Transforms/InstCombine/tbaa-store-to-load.ll | 17 +++++++++++++++++
4 files changed, 27 insertions(+), 4 deletions(-)
create mode 100644 test/Transforms/InstCombine/tbaa-store-to-load.ll

diff --git a/include/llvm/Analysis/Loads.h b/include/llvm/Analysis/Loads.h
index 42667d2..5db3c2f 100644
--- a/include/llvm/Analysis/Loads.h
+++ b/include/llvm/Analysis/Loads.h
@@ -50,7 +50,8 @@ Value *FindAvailableLoadedValue(Value *Ptr, BasicBlock *ScanBB,
BasicBlock::iterator &ScanFrom,
unsigned MaxInstsToScan = 6,
AliasAnalysis *AA = nullptr,
- AAMDNodes *AATags = nullptr);
+ AAMDNodes *AATags = nullptr,
+ bool *IsLoadCSE = nullptr);

}

diff --git a/lib/Analysis/Loads.cpp b/lib/Analysis/Loads.cpp
index 624c5a1..baecd3f 100644
--- a/lib/Analysis/Loads.cpp
+++ b/lib/Analysis/Loads.cpp
@@ -183,7 +183,8 @@ bool llvm::isSafeToLoadUnconditionally(Value *V, Instruction *ScanFrom,
Value *llvm::FindAvailableLoadedValue(Value *Ptr, BasicBlock *ScanBB,
BasicBlock::iterator &ScanFrom,
unsigned MaxInstsToScan,
- AliasAnalysis *AA, AAMDNodes *AATags) {
+ AliasAnalysis *AA, AAMDNodes *AATags,
+ bool *IsLoadCSE) {
if (MaxInstsToScan == 0)
MaxInstsToScan = ~0U;

@@ -220,6 +221,8 @@ Value *llvm::FindAvailableLoadedValue(Value *Ptr, BasicBlock *ScanBB,
CastInst::isBitOrNoopPointerCastable(LI->getType(), AccessTy, DL)) {
if (AATags)
LI->getAAMetadata(*AATags);
+ if (IsLoadCSE)
+ *IsLoadCSE = true;
return LI;
}

diff --git a/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
index e3179db..f1af1f7 100644
--- a/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ b/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -750,9 +750,11 @@ Instruction *InstCombiner::visitLoadInst(LoadInst &LI) {
// separated by a few arithmetic operations.
BasicBlock::iterator BBI = &LI;
AAMDNodes AATags;
+ bool IsLoadCSE = false;
if (Value *AvailableVal = FindAvailableLoadedValue(Op, LI.getParent(), BBI,
- 6, AA, &AATags)) {
- if (LoadInst *NLI = dyn_cast<LoadInst>(AvailableVal)) {
+ 6, AA, &AATags, &IsLoadCSE)) {
+ if (IsLoadCSE) {
+ LoadInst *NLI = cast<LoadInst>(AvailableVal);
unsigned KnownIDs[] = {
LLVMContext::MD_tbaa,
LLVMContext::MD_alias_scope,
diff --git a/test/Transforms/InstCombine/tbaa-store-to-load.ll b/test/Transforms/InstCombine/tbaa-store-to-load.ll
new file mode 100644
index 0000000..707be73
--- /dev/null
+++ b/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"}
--
2.8.3

98 changes: 98 additions & 0 deletions deps/patches/llvm-D21271-instcombine-tbaa-3.8.patch
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
From 3dc2f7e69ec0a65af503bb6264fa6674ecfeaf84 Mon Sep 17 00:00:00 2001
From: Yichao Yu <[email protected]>
Date: Sun, 12 Jun 2016 17:14:38 -0400
Subject: [PATCH] Fix `InstCombine` to not widen metadata on store-to-load
forwarding.

The original check for load CSE or store-to-load forwarding is wrong
when the forwarded stored value happened to be a load.
---
include/llvm/Analysis/Loads.h | 3 ++-
lib/Analysis/Loads.cpp | 5 ++++-
.../InstCombine/InstCombineLoadStoreAlloca.cpp | 6 ++++--
test/Transforms/InstCombine/tbaa-store-to-load.ll | 17 +++++++++++++++++
4 files changed, 27 insertions(+), 4 deletions(-)
create mode 100644 test/Transforms/InstCombine/tbaa-store-to-load.ll

diff --git a/include/llvm/Analysis/Loads.h b/include/llvm/Analysis/Loads.h
index 939663b..a7fc035 100644
--- a/include/llvm/Analysis/Loads.h
+++ b/include/llvm/Analysis/Loads.h
@@ -55,7 +55,8 @@ Value *FindAvailableLoadedValue(Value *Ptr, BasicBlock *ScanBB,
BasicBlock::iterator &ScanFrom,
unsigned MaxInstsToScan = DefMaxInstsToScan,
AliasAnalysis *AA = nullptr,
- AAMDNodes *AATags = nullptr);
+ AAMDNodes *AATags = nullptr,
+ bool *IsLoadCSE = nullptr);

}

diff --git a/lib/Analysis/Loads.cpp b/lib/Analysis/Loads.cpp
index 4b2fa3c..f5705fa 100644
--- a/lib/Analysis/Loads.cpp
+++ b/lib/Analysis/Loads.cpp
@@ -196,7 +196,8 @@ llvm::DefMaxInstsToScan("available-load-scan-limit", cl::init(6), cl::Hidden,
Value *llvm::FindAvailableLoadedValue(Value *Ptr, BasicBlock *ScanBB,
BasicBlock::iterator &ScanFrom,
unsigned MaxInstsToScan,
- AliasAnalysis *AA, AAMDNodes *AATags) {
+ AliasAnalysis *AA, AAMDNodes *AATags,
+ bool *IsLoadCSE) {
if (MaxInstsToScan == 0)
MaxInstsToScan = ~0U;

@@ -233,6 +234,8 @@ Value *llvm::FindAvailableLoadedValue(Value *Ptr, BasicBlock *ScanBB,
CastInst::isBitOrNoopPointerCastable(LI->getType(), AccessTy, DL)) {
if (AATags)
LI->getAAMetadata(*AATags);
+ if (IsLoadCSE)
+ *IsLoadCSE = true;
return LI;
}

diff --git a/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
index 4d42658..8302ef8 100644
--- a/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ b/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -800,10 +800,12 @@ Instruction *InstCombiner::visitLoadInst(LoadInst &LI) {
// separated by a few arithmetic operations.
BasicBlock::iterator BBI(LI);
AAMDNodes AATags;
+ bool IsLoadCSE = false;
if (Value *AvailableVal =
FindAvailableLoadedValue(Op, LI.getParent(), BBI,
- DefMaxInstsToScan, AA, &AATags)) {
- if (LoadInst *NLI = dyn_cast<LoadInst>(AvailableVal)) {
+ DefMaxInstsToScan, AA, &AATags, &IsLoadCSE)) {
+ if (IsLoadCSE) {
+ LoadInst *NLI = cast<LoadInst>(AvailableVal);
unsigned KnownIDs[] = {
LLVMContext::MD_tbaa, LLVMContext::MD_alias_scope,
LLVMContext::MD_noalias, LLVMContext::MD_range,
diff --git a/test/Transforms/InstCombine/tbaa-store-to-load.ll b/test/Transforms/InstCombine/tbaa-store-to-load.ll
new file mode 100644
index 0000000..707be73
--- /dev/null
+++ b/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"}
--
2.8.3

0 comments on commit b03c3e3

Please sign in to comment.