Skip to content

Commit

Permalink
Merge pull request #16503 from JuliaLang/yyc/llvm38
Browse files Browse the repository at this point in the history
Fix SegFault on LLVM 3.8.0
  • Loading branch information
tkelman committed May 22, 2016
2 parents fd559d3 + 2c394f4 commit 6497786
Show file tree
Hide file tree
Showing 2 changed files with 153 additions and 0 deletions.
3 changes: 3 additions & 0 deletions deps/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -728,6 +728,9 @@ $(eval $(call LLVM_PATCH,llvm-3.8.0_winshlib))
$(eval $(call LLVM_PATCH,llvm-3.8.0_threads))
# fix replutil test on unix
$(eval $(call LLVM_PATCH,llvm-D17165-D18583))
# Segfault for aggregate load
$(eval $(call LLVM_PATCH,llvm-D17326_unpack_load))
$(LLVM_SRC_DIR)/llvm-D17326_unpack_load.patch-applied: $(LLVM_SRC_DIR)/llvm-D14260.patch-applied
$(eval $(call LLVM_PATCH,llvm-D17712))
$(eval $(call LLVM_PATCH,llvm-PR26180))
$(eval $(call LLVM_PATCH,llvm-PR27046))
Expand Down
150 changes: 150 additions & 0 deletions deps/llvm-D17326_unpack_load.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
From 92c0b571a3a7420bdc3dfad72b4c75dde725d9a9 Mon Sep 17 00:00:00 2001
From: Amaury Sechet <deadalnix@gmail.com>
Date: Wed, 17 Feb 2016 19:21:28 +0000
Subject: [PATCH] Fix load alignement when unpacking aggregates structs

Summary: Store and loads unpacked by instcombine do not always have the right alignement. This explicitely compute the alignement and set it.

Reviewers: dblaikie, majnemer, reames, hfinkel, joker.eph

Subscribers: llvm-commits

Differential Revision: http://reviews.llvm.org/D17326

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@261139 91177308-0d34-0410-b5e6-96231b3b80d8
---
.../InstCombine/InstCombineLoadStoreAlloca.cpp | 38 +++++++++++++++-------
test/Transforms/InstCombine/unpack-fca.ll | 27 +++++++++++++++
2 files changed, 53 insertions(+), 12 deletions(-)

diff --git a/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
index dd2889d..4d42658 100644
--- a/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ b/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -523,16 +523,17 @@ static Instruction *unpackLoadToAggregate(InstCombiner &IC, LoadInst &LI) {
if (!T->isAggregateType())
return nullptr;

+ auto Name = LI.getName();
assert(LI.getAlignment() && "Alignment must be set at this point");

if (auto *ST = dyn_cast<StructType>(T)) {
// If the struct only have one element, we unpack.
- unsigned Count = ST->getNumElements();
- if (Count == 1) {
+ auto NumElements = ST->getNumElements();
+ if (NumElements == 1) {
LoadInst *NewLoad = combineLoadToNewType(IC, LI, ST->getTypeAtIndex(0U),
".unpack");
return IC.ReplaceInstUsesWith(LI, IC.Builder->CreateInsertValue(
- UndefValue::get(T), NewLoad, 0, LI.getName()));
+ UndefValue::get(T), NewLoad, 0, Name));
}

// We don't want to break loads with padding here as we'd loose
@@ -542,23 +543,29 @@ static Instruction *unpackLoadToAggregate(InstCombiner &IC, LoadInst &LI) {
if (SL->hasPadding())
return nullptr;

- auto Name = LI.getName();
+ auto Align = LI.getAlignment();
+ if (!Align)
+ Align = DL.getABITypeAlignment(ST);
+
SmallString<16> LoadName = Name;
LoadName += ".unpack";
SmallString<16> EltName = Name;
EltName += ".elt";
+
auto *Addr = LI.getPointerOperand();
- Value *V = UndefValue::get(T);
- auto *IdxType = Type::getInt32Ty(ST->getContext());
+ auto *IdxType = Type::getInt32Ty(T->getContext());
auto *Zero = ConstantInt::get(IdxType, 0);
- for (unsigned i = 0; i < Count; i++) {
+
+ Value *V = UndefValue::get(T);
+ for (unsigned i = 0; i < NumElements; i++) {
Value *Indices[2] = {
Zero,
ConstantInt::get(IdxType, i),
};
- auto *Ptr = IC.Builder->CreateInBoundsGEP(ST, Addr, makeArrayRef(Indices), EltName);
- auto *L = IC.Builder->CreateAlignedLoad(Ptr, LI.getAlignment(),
- LoadName);
+ auto *Ptr = IC.Builder->CreateInBoundsGEP(ST, Addr,
+ makeArrayRef(Indices), EltName);
+ auto EltAlign = MinAlign(Align, SL->getElementOffset(i));
+ auto *L = IC.Builder->CreateAlignedLoad(Ptr, EltAlign, LoadName);
V = IC.Builder->CreateInsertValue(V, L, i);
}

@@ -950,11 +957,16 @@ static bool unpackStoreToAggregate(InstCombiner &IC, StoreInst &SI) {
if (SL->hasPadding())
return false;

+ auto Align = SI.getAlignment();
+ if (!Align)
+ Align = DL.getABITypeAlignment(ST);
+
SmallString<16> EltName = V->getName();
EltName += ".elt";
auto *Addr = SI.getPointerOperand();
SmallString<16> AddrName = Addr->getName();
AddrName += ".repack";
+
auto *IdxType = Type::getInt32Ty(ST->getContext());
auto *Zero = ConstantInt::get(IdxType, 0);
for (unsigned i = 0; i < Count; i++) {
@@ -962,9 +974,11 @@ static bool unpackStoreToAggregate(InstCombiner &IC, StoreInst &SI) {
Zero,
ConstantInt::get(IdxType, i),
};
- auto *Ptr = IC.Builder->CreateInBoundsGEP(ST, Addr, makeArrayRef(Indices), AddrName);
+ auto *Ptr = IC.Builder->CreateInBoundsGEP(ST, Addr,
+ makeArrayRef(Indices), AddrName);
auto *Val = IC.Builder->CreateExtractValue(V, i, EltName);
- IC.Builder->CreateStore(Val, Ptr);
+ auto EltAlign = MinAlign(Align, SL->getElementOffset(i));
+ IC.Builder->CreateAlignedStore(Val, Ptr, EltAlign);
}

return true;
diff --git a/test/Transforms/InstCombine/unpack-fca.ll b/test/Transforms/InstCombine/unpack-fca.ll
index 4359839..bed3b61 100644
--- a/test/Transforms/InstCombine/unpack-fca.ll
+++ b/test/Transforms/InstCombine/unpack-fca.ll
@@ -151,3 +151,30 @@ define i32 @packed_alignment(%struct.S* dereferenceable(9) %s) {
%v = extractvalue %struct.T %tv, 1
ret i32 %v
}
+
+%struct.U = type {i8, i8, i8, i8, i8, i8, i8, i8, i64}
+
+define void @check_alignment(%struct.U* %u, %struct.U* %v) {
+; CHECK-LABEL: check_alignment
+; CHECK: load i8, i8* {{.*}}, align 8
+; CHECK: load i8, i8* {{.*}}, align 1
+; CHECK: load i8, i8* {{.*}}, align 2
+; CHECK: load i8, i8* {{.*}}, align 1
+; CHECK: load i8, i8* {{.*}}, align 4
+; CHECK: load i8, i8* {{.*}}, align 1
+; CHECK: load i8, i8* {{.*}}, align 2
+; CHECK: load i8, i8* {{.*}}, align 1
+; CHECK: load i64, i64* {{.*}}, align 8
+; CHECK: store i8 {{.*}}, i8* {{.*}}, align 8
+; CHECK: store i8 {{.*}}, i8* {{.*}}, align 1
+; CHECK: store i8 {{.*}}, i8* {{.*}}, align 2
+; CHECK: store i8 {{.*}}, i8* {{.*}}, align 1
+; CHECK: store i8 {{.*}}, i8* {{.*}}, align 4
+; CHECK: store i8 {{.*}}, i8* {{.*}}, align 1
+; CHECK: store i8 {{.*}}, i8* {{.*}}, align 2
+; CHECK: store i8 {{.*}}, i8* {{.*}}, align 1
+; CHECK: store i64 {{.*}}, i64* {{.*}}, align 8
+ %1 = load %struct.U, %struct.U* %u
+ store %struct.U %1, %struct.U* %v
+ ret void
+}
--
2.8.2

0 comments on commit 6497786

Please sign in to comment.