r216722 - Use store size instead of alloc size when coercing.

James Molloy james.molloy at arm.com
Fri Aug 29 03:17:52 PDT 2014


Author: jamesm
Date: Fri Aug 29 05:17:52 2014
New Revision: 216722

URL: http://llvm.org/viewvc/llvm-project?rev=216722&view=rev
Log:
Use store size instead of alloc size when coercing.

Previously, EnterStructPointerForCoercedAccess used Alloc size when determining how to convert. This was problematic, because there were situations were the alloc size was larger than the store size. For example, if the first element of a structure were i24 and the destination type were i32, the old code would generate a GEP and a load i24. The code should compare store sizes to ensure the whole object is loaded. I have attached a test case.

This patch modifies the output of arm64-be-bitfield.c test case, but the new IR seems to be equivalent, and after -O3, the compiler generates identical ARM assembly. (asr x0, x0, #54)

Patch by Thomas Jablin!


Added:
    cfe/trunk/test/CodeGen/24-bit.c
Modified:
    cfe/trunk/lib/CodeGen/CGCall.cpp
    cfe/trunk/test/CodeGen/arm64-be-bitfield.c

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=216722&r1=216721&r2=216722&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Fri Aug 29 05:17:52 2014
@@ -627,11 +627,13 @@ EnterStructPointerForCoercedAccess(llvm:
   llvm::Type *FirstElt = SrcSTy->getElementType(0);
 
   // If the first elt is at least as large as what we're looking for, or if the
-  // first element is the same size as the whole struct, we can enter it.
+  // first element is the same size as the whole struct, we can enter it. The
+  // comparison must be made on the store size and not the alloca size. Using
+  // the alloca size may overstate the size of the load.
   uint64_t FirstEltSize =
-    CGF.CGM.getDataLayout().getTypeAllocSize(FirstElt);
+    CGF.CGM.getDataLayout().getTypeStoreSize(FirstElt);
   if (FirstEltSize < DstSize &&
-      FirstEltSize < CGF.CGM.getDataLayout().getTypeAllocSize(SrcSTy))
+      FirstEltSize < CGF.CGM.getDataLayout().getTypeStoreSize(SrcSTy))
     return SrcPtr;
 
   // GEP into the first element.

Added: cfe/trunk/test/CodeGen/24-bit.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/24-bit.c?rev=216722&view=auto
==============================================================================
--- cfe/trunk/test/CodeGen/24-bit.c (added)
+++ cfe/trunk/test/CodeGen/24-bit.c Fri Aug 29 05:17:52 2014
@@ -0,0 +1,14 @@
+// RUN:  %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -O0 -o - %s | FileCheck %s
+
+static union ibtt2
+{
+  struct ibtt0 { signed ibt0:10; unsigned short ibt1; } ibt5;
+  struct ibtt1 { signed ibt2:3; signed ibt3:9; signed ibt4:9; } ibt6;
+} ibt15 = {{267, 15266}};
+
+void callee_ibt0f(union ibtt2 ibtp5);
+
+void test(void) {
+// CHECK: = load i32*
+  callee_ibt0f(ibt15);
+}

Modified: cfe/trunk/test/CodeGen/arm64-be-bitfield.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/arm64-be-bitfield.c?rev=216722&r1=216721&r2=216722&view=diff
==============================================================================
--- cfe/trunk/test/CodeGen/arm64-be-bitfield.c (original)
+++ cfe/trunk/test/CodeGen/arm64-be-bitfield.c Fri Aug 29 05:17:52 2014
@@ -1,9 +1,15 @@
-// RUN:  %clang_cc1 -triple aarch64_be-linux-gnu -ffreestanding -emit-llvm -O0 -o - %s | FileCheck %s
+// REQUIRES: aarch64-registered-target
+// RUN:  %clang_cc1 -triple aarch64_be-linux-gnu -ffreestanding -emit-llvm -O0 -o - %s | FileCheck --check-prefix IR %s
+// RUN:  %clang_cc1 -triple aarch64_be-linux-gnu -ffreestanding -S -O1 -o - %s | FileCheck --check-prefix ARM %s
 
 struct bt3 { signed b2:10; signed b3:10; } b16;
 
-// The correct right-shift amount is 40 bits for big endian.
+// Get the high 32-bits and then shift appropriately for big-endian.
 signed callee_b0f(struct bt3 bp11) {
-// CHECK: = lshr i64 %{{.*}}, 40
+// IR: callee_b0f(i64 [[ARG:%.*]])
+// IR: store i64 [[ARG]], i64* [[PTR:%.*]]
+// IR: [[BITCAST:%.*]] = bitcast i64* [[PTR]] to i8*
+// IR: call void @llvm.memcpy.p0i8.p0i8.i64(i8* {{.*}}, i8* [[BITCAST]], i64 4
+// ARM: asr x0, x0, #54
   return bp11.b2;
 }





More information about the cfe-commits mailing list