[PATCH] D17158: Don't promote load alignment from packed structure elements

[Pete Cooper via llvm-commits]

pete created this revision.
pete added reviewers: majnemer, deadalnix.
pete added subscribers: hans, llvm-commits.
pete set the repository for this revision to rL LLVM.

Hi all

Looks like http://reviews.llvm.org/D8339 breaks loads from packed structs.

The problem is that when we create a new load of the aggregate type, we didn't set its alignment.  So a load of a packed int gets promote from align 1 to align 4.

I was told about this offline by Neil Henning from Codeplay.  He was able to describe the problem well enough that I could make this small test case.

He also mentioned that this is breaking his code in a way where he would really like to see this get in to 3.8.  No idea if thats possible, but just putting that out there to see if that would be ok.

Thanks,
Pete

Repository:
  rL LLVM

http://reviews.llvm.org/D17158

Files:
  lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
  test/Transforms/InstCombine/unpack-fca.ll

Index: test/Transforms/InstCombine/unpack-fca.ll
===================================================================
--- test/Transforms/InstCombine/unpack-fca.ll
+++ test/Transforms/InstCombine/unpack-fca.ll
@@ -136,3 +136,18 @@
   %1 = load %B, %B* %b.ptr, align 8
   ret %B %1
 }
+
+%struct.S = type <{ i8, %struct.T }>
+%struct.T = type { i32, i32 }
+
+; Make sure that we do not increase alignment of packed struct element
+define i32 @packed_alignment(%struct.S* dereferenceable(9) %s) {
+; CHECK-LABEL: packed_alignment
+; CHECK-NEXT: %tv.elt1 = getelementptr inbounds %struct.S, %struct.S* %s, i64 0, i32 1, i32 1
+; CHECK-NEXT: %tv.unpack2 = load i32, i32* %tv.elt1, align 1
+; CHECK-NEXT: ret i32 %tv.unpack2
+  %t = getelementptr inbounds %struct.S, %struct.S* %s, i32 0, i32 1
+  %tv = load %struct.T, %struct.T* %t, align 1
+  %v = extractvalue %struct.T %tv, 1
+  ret i32 %v
+}
Index: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
===================================================================
--- lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -552,7 +552,8 @@
         ConstantInt::get(IdxType, i),
       };
       auto *Ptr = IC.Builder->CreateInBoundsGEP(ST, Addr, makeArrayRef(Indices), EltName);
-      auto *L = IC.Builder->CreateLoad(ST->getTypeAtIndex(i), Ptr, LoadName);
+      auto *L = IC.Builder->CreateAlignedLoad(Ptr, LI.getAlignment(),
+                                              LoadName);
       V = IC.Builder->CreateInsertValue(V, L, i);
     }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D17158.47690.patch
Type: text/x-patch
Size: 1578 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160211/8e9401b0/attachment.bin>