From fd8eca202f517a65f9668990ecdc9c710406ff9e Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Sun, 9 Dec 2012 07:26:04 +0000 Subject: [PATCH] Fix the bitfield record layout in codegen for big endian targets. This was an egregious bug due to the several iterations of refactorings that took place. Size no longer meant what it original did by the time I finished, but this line of code never got updated. Unfortunately we had essentially zero tests for this in the regression test suite. =[ I've added a PPC64 run over the bitfield test case I've been primarily using. I'm still looking at adding more tests and making sure this is the *correct* bitfield access code on PPC64 linux, but it looks pretty close to me, and it is *worlds* better than before this patch as it no longer asserts! =] More commits to follow with at least additional tests and maybe more fixes. Sorry for the long breakage due to this.... llvm-svn: 169691 --- clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 10 +- clang/test/CodeGenCXX/bitfield.cpp | 238 +++++++++++++------- 2 files changed, 169 insertions(+), 79 deletions(-) diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp index 123848b8fce2..92c5672c732c 100644 --- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp +++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp @@ -256,9 +256,13 @@ CGBitFieldInfo CGBitFieldInfo::MakeInfo(CodeGenTypes &Types, Size = TypeSizeInBits; } - // Reverse the bit offsets for big endian machines. - if (Types.getDataLayout().isBigEndian()) - Offset = Size - Offset - 1; + // Reverse the bit offsets for big endian machines. Because we represent + // a bitfield as a single large integer load, we can imagine the bits + // counting from the most-significant-bit instead of the + // least-significant-bit. + if (Types.getDataLayout().isBigEndian()) { + Offset = StorageSize - (Offset + Size); + } return CGBitFieldInfo(Offset, Size, IsSigned, StorageSize, StorageAlignment); } diff --git a/clang/test/CodeGenCXX/bitfield.cpp b/clang/test/CodeGenCXX/bitfield.cpp index a737304c98d4..49471d245a8b 100644 --- a/clang/test/CodeGenCXX/bitfield.cpp +++ b/clang/test/CodeGenCXX/bitfield.cpp @@ -1,4 +1,7 @@ -// RUN: %clang_cc1 -triple x86_64-unknown-unknown -verify -emit-llvm -o - %s | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -verify -emit-llvm -o - %s \ +// RUN: | FileCheck -check-prefix=CHECK-X86-64 %s +// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -verify -emit-llvm -o - %s \ +// RUN: | FileCheck -check-prefix=CHECK-PPC64 %s // // Tests for bitfield access patterns in C++ with special attention to // conformance to C++11 memory model requirements. @@ -16,23 +19,38 @@ namespace N1 { char c; }; unsigned read(S* s) { - // CHECK: define i32 @_ZN2N14read - // CHECK: %[[ptr:.*]] = getelementptr inbounds %{{.*}}* %{{.*}}, i32 0, i32 1 - // CHECK: %[[val:.*]] = load i8* %[[ptr]] - // CHECK: %[[and:.*]] = and i8 %[[val]], 1 - // CHECK: %[[ext:.*]] = zext i8 %[[and]] to i32 - // CHECK: ret i32 %[[ext]] + // CHECK-X86-64: define i32 @_ZN2N14read + // CHECK-X86-64: %[[ptr:.*]] = getelementptr inbounds %{{.*}}* %{{.*}}, i32 0, i32 1 + // CHECK-X86-64: %[[val:.*]] = load i8* %[[ptr]] + // CHECK-X86-64: %[[and:.*]] = and i8 %[[val]], 1 + // CHECK-X86-64: %[[ext:.*]] = zext i8 %[[and]] to i32 + // CHECK-X86-64: ret i32 %[[ext]] + // CHECK-PPC64: define zeroext i32 @_ZN2N14read + // CHECK-PPC64: %[[ptr:.*]] = getelementptr inbounds %{{.*}}* %{{.*}}, i32 0, i32 1 + // CHECK-PPC64: %[[val:.*]] = load i8* %[[ptr]] + // CHECK-PPC64: %[[shr:.*]] = lshr i8 %[[val]], 7 + // CHECK-PPC64: %[[ext:.*]] = zext i8 %[[shr]] to i32 + // CHECK-PPC64: ret i32 %[[ext]] return s->b; } void write(S* s, unsigned x) { - // CHECK: define void @_ZN2N15write - // CHECK: %[[ptr:.*]] = getelementptr inbounds %{{.*}}* %{{.*}}, i32 0, i32 1 - // CHECK: %[[x_trunc:.*]] = trunc i32 %{{.*}} to i8 - // CHECK: %[[old:.*]] = load i8* %[[ptr]] - // CHECK: %[[x_and:.*]] = and i8 %[[x_trunc]], 1 - // CHECK: %[[old_and:.*]] = and i8 %[[old]], -2 - // CHECK: %[[new:.*]] = or i8 %[[old_and]], %[[x_and]] - // CHECK: store i8 %[[new]], i8* %[[ptr]] + // CHECK-X86-64: define void @_ZN2N15write + // CHECK-X86-64: %[[ptr:.*]] = getelementptr inbounds %{{.*}}* %{{.*}}, i32 0, i32 1 + // CHECK-X86-64: %[[x_trunc:.*]] = trunc i32 %{{.*}} to i8 + // CHECK-X86-64: %[[old:.*]] = load i8* %[[ptr]] + // CHECK-X86-64: %[[x_and:.*]] = and i8 %[[x_trunc]], 1 + // CHECK-X86-64: %[[old_and:.*]] = and i8 %[[old]], -2 + // CHECK-X86-64: %[[new:.*]] = or i8 %[[old_and]], %[[x_and]] + // CHECK-X86-64: store i8 %[[new]], i8* %[[ptr]] + // CHECK-PPC64: define void @_ZN2N15write + // CHECK-PPC64: %[[ptr:.*]] = getelementptr inbounds %{{.*}}* %{{.*}}, i32 0, i32 1 + // CHECK-PPC64: %[[x_trunc:.*]] = trunc i32 %{{.*}} to i8 + // CHECK-PPC64: %[[old:.*]] = load i8* %[[ptr]] + // CHECK-PPC64: %[[x_and:.*]] = and i8 %[[x_trunc]], 1 + // CHECK-PPC64: %[[x_shl:.*]] = shl i8 %[[x_and]], 7 + // CHECK-PPC64: %[[old_and:.*]] = and i8 %[[old]], 127 + // CHECK-PPC64: %[[new:.*]] = or i8 %[[old_and]], %[[x_shl]] + // CHECK-PPC64: store i8 %[[new]], i8* %[[ptr]] s->b = x; } } @@ -45,21 +63,34 @@ namespace N2 { void *p; }; unsigned read(S* s) { - // CHECK: define i32 @_ZN2N24read - // CHECK: %[[ptr:.*]] = bitcast %{{.*}}* %{{.*}} to i32* - // CHECK: %[[val:.*]] = load i32* %[[ptr]] - // CHECK: %[[and:.*]] = and i32 %[[val]], 16777215 - // CHECK: ret i32 %[[and]] + // CHECK-X86-64: define i32 @_ZN2N24read + // CHECK-X86-64: %[[ptr:.*]] = bitcast %{{.*}}* %{{.*}} to i32* + // CHECK-X86-64: %[[val:.*]] = load i32* %[[ptr]] + // CHECK-X86-64: %[[and:.*]] = and i32 %[[val]], 16777215 + // CHECK-X86-64: ret i32 %[[and]] + // CHECK-PPC64: define zeroext i32 @_ZN2N24read + // CHECK-PPC64: %[[ptr:.*]] = bitcast %{{.*}}* %{{.*}} to i32* + // CHECK-PPC64: %[[val:.*]] = load i32* %[[ptr]] + // CHECK-PPC64: %[[shr:.*]] = lshr i32 %[[val]], 8 + // CHECK-PPC64: ret i32 %[[shr]] return s->b; } void write(S* s, unsigned x) { - // CHECK: define void @_ZN2N25write - // CHECK: %[[ptr:.*]] = bitcast %{{.*}}* %{{.*}} to i32* - // CHECK: %[[old:.*]] = load i32* %[[ptr]] - // CHECK: %[[x_and:.*]] = and i32 %{{.*}}, 16777215 - // CHECK: %[[old_and:.*]] = and i32 %[[old]], -16777216 - // CHECK: %[[new:.*]] = or i32 %[[old_and]], %[[x_and]] - // CHECK: store i32 %[[new]], i32* %[[ptr]] + // CHECK-X86-64: define void @_ZN2N25write + // CHECK-X86-64: %[[ptr:.*]] = bitcast %{{.*}}* %{{.*}} to i32* + // CHECK-X86-64: %[[old:.*]] = load i32* %[[ptr]] + // CHECK-X86-64: %[[x_and:.*]] = and i32 %{{.*}}, 16777215 + // CHECK-X86-64: %[[old_and:.*]] = and i32 %[[old]], -16777216 + // CHECK-X86-64: %[[new:.*]] = or i32 %[[old_and]], %[[x_and]] + // CHECK-X86-64: store i32 %[[new]], i32* %[[ptr]] + // CHECK-PPC64: define void @_ZN2N25write + // CHECK-PPC64: %[[ptr:.*]] = bitcast %{{.*}}* %{{.*}} to i32* + // CHECK-PPC64: %[[old:.*]] = load i32* %[[ptr]] + // CHECK-PPC64: %[[x_and:.*]] = and i32 %{{.*}}, 16777215 + // CHECK-PPC64: %[[x_shl:.*]] = shl i32 %[[x_and]], 8 + // CHECK-PPC64: %[[old_and:.*]] = and i32 %[[old]], 255 + // CHECK-PPC64: %[[new:.*]] = or i32 %[[old_and]], %[[x_shl]] + // CHECK-PPC64: store i32 %[[new]], i32* %[[ptr]] s->b = x; } } @@ -71,21 +102,34 @@ namespace N3 { unsigned b : 24; }; unsigned read(S* s) { - // CHECK: define i32 @_ZN2N34read - // CHECK: %[[ptr:.*]] = bitcast %{{.*}}* %{{.*}} to i32* - // CHECK: %[[val:.*]] = load i32* %[[ptr]] - // CHECK: %[[and:.*]] = and i32 %[[val]], 16777215 - // CHECK: ret i32 %[[and]] + // CHECK-X86-64: define i32 @_ZN2N34read + // CHECK-X86-64: %[[ptr:.*]] = bitcast %{{.*}}* %{{.*}} to i32* + // CHECK-X86-64: %[[val:.*]] = load i32* %[[ptr]] + // CHECK-X86-64: %[[and:.*]] = and i32 %[[val]], 16777215 + // CHECK-X86-64: ret i32 %[[and]] + // CHECK-PPC64: define zeroext i32 @_ZN2N34read + // CHECK-PPC64: %[[ptr:.*]] = bitcast %{{.*}}* %{{.*}} to i32* + // CHECK-PPC64: %[[val:.*]] = load i32* %[[ptr]] + // CHECK-PPC64: %[[shr:.*]] = lshr i32 %[[val]], 8 + // CHECK-PPC64: ret i32 %[[shr]] return s->b; } void write(S* s, unsigned x) { - // CHECK: define void @_ZN2N35write - // CHECK: %[[ptr:.*]] = bitcast %{{.*}}* %{{.*}} to i32* - // CHECK: %[[old:.*]] = load i32* %[[ptr]] - // CHECK: %[[x_and:.*]] = and i32 %{{.*}}, 16777215 - // CHECK: %[[old_and:.*]] = and i32 %[[old]], -16777216 - // CHECK: %[[new:.*]] = or i32 %[[old_and]], %[[x_and]] - // CHECK: store i32 %[[new]], i32* %[[ptr]] + // CHECK-X86-64: define void @_ZN2N35write + // CHECK-X86-64: %[[ptr:.*]] = bitcast %{{.*}}* %{{.*}} to i32* + // CHECK-X86-64: %[[old:.*]] = load i32* %[[ptr]] + // CHECK-X86-64: %[[x_and:.*]] = and i32 %{{.*}}, 16777215 + // CHECK-X86-64: %[[old_and:.*]] = and i32 %[[old]], -16777216 + // CHECK-X86-64: %[[new:.*]] = or i32 %[[old_and]], %[[x_and]] + // CHECK-X86-64: store i32 %[[new]], i32* %[[ptr]] + // CHECK-PPC64: define void @_ZN2N35write + // CHECK-PPC64: %[[ptr:.*]] = bitcast %{{.*}}* %{{.*}} to i32* + // CHECK-PPC64: %[[old:.*]] = load i32* %[[ptr]] + // CHECK-PPC64: %[[x_and:.*]] = and i32 %{{.*}}, 16777215 + // CHECK-PPC64: %[[x_shl:.*]] = shl i32 %[[x_and]], 8 + // CHECK-PPC64: %[[old_and:.*]] = and i32 %[[old]], 255 + // CHECK-PPC64: %[[new:.*]] = or i32 %[[old_and]], %[[x_shl]] + // CHECK-PPC64: store i32 %[[new]], i32* %[[ptr]] s->b = x; } } @@ -109,18 +153,31 @@ namespace N4 { // FIXME: We should widen this load as long as the function isn't being // instrumented by thread-sanitizer. // - // CHECK: define i32 @_ZN2N44read - // CHECK: %[[ptr:.*]] = bitcast {{.*}}* %{{.*}} to i24* - // CHECK: %[[val:.*]] = load i24* %[[ptr]] - // CHECK: %[[ext:.*]] = zext i24 %[[val]] to i32 - // CHECK: ret i32 %[[ext]] + // CHECK-X86-64: define i32 @_ZN2N44read + // CHECK-X86-64: %[[gep:.*]] = getelementptr inbounds {{.*}}* %{{.*}}, i32 0, i32 1 + // CHECK-X86-64: %[[ptr:.*]] = bitcast [3 x i8]* %[[gep]] to i24* + // CHECK-X86-64: %[[val:.*]] = load i24* %[[ptr]] + // CHECK-X86-64: %[[ext:.*]] = zext i24 %[[val]] to i32 + // CHECK-X86-64: ret i32 %[[ext]] + // CHECK-PPC64: define zeroext i32 @_ZN2N44read + // CHECK-PPC64: %[[gep:.*]] = getelementptr inbounds {{.*}}* %{{.*}}, i32 0, i32 1 + // CHECK-PPC64: %[[ptr:.*]] = bitcast [3 x i8]* %[[gep]] to i24* + // CHECK-PPC64: %[[val:.*]] = load i24* %[[ptr]] + // CHECK-PPC64: %[[ext:.*]] = zext i24 %[[val]] to i32 + // CHECK-PPC64: ret i32 %[[ext]] return s->b; } void write(Base* s, unsigned x) { - // CHECK: define void @_ZN2N45write - // CHECK: %[[ptr:.*]] = bitcast {{.*}}* %{{.*}} to i24* - // CHECK: %[[new:.*]] = trunc i32 %{{.*}} to i24 - // CHECK: store i24 %[[new]], i24* %[[ptr]] + // CHECK-X86-64: define void @_ZN2N45write + // CHECK-X86-64: %[[gep:.*]] = getelementptr inbounds {{.*}}* %{{.*}}, i32 0, i32 1 + // CHECK-X86-64: %[[ptr:.*]] = bitcast [3 x i8]* %[[gep]] to i24* + // CHECK-X86-64: %[[new:.*]] = trunc i32 %{{.*}} to i24 + // CHECK-X86-64: store i24 %[[new]], i24* %[[ptr]] + // CHECK-PPC64: define void @_ZN2N45write + // CHECK-PPC64: %[[gep:.*]] = getelementptr inbounds {{.*}}* %{{.*}}, i32 0, i32 1 + // CHECK-PPC64: %[[ptr:.*]] = bitcast [3 x i8]* %[[gep]] to i24* + // CHECK-PPC64: %[[new:.*]] = trunc i32 %{{.*}} to i24 + // CHECK-PPC64: store i24 %[[new]], i24* %[[ptr]] s->b = x; } } @@ -138,21 +195,34 @@ namespace N5 { struct Y { unsigned b : 24; } y; }; unsigned read(U* u) { - // CHECK: define i32 @_ZN2N54read - // CHECK: %[[ptr:.*]] = bitcast %{{.*}}* %{{.*}} to i32* - // CHECK: %[[val:.*]] = load i32* %[[ptr]] - // CHECK: %[[and:.*]] = and i32 %[[val]], 16777215 - // CHECK: ret i32 %[[and]] + // CHECK-X86-64: define i32 @_ZN2N54read + // CHECK-X86-64: %[[ptr:.*]] = bitcast %{{.*}}* %{{.*}} to i32* + // CHECK-X86-64: %[[val:.*]] = load i32* %[[ptr]] + // CHECK-X86-64: %[[and:.*]] = and i32 %[[val]], 16777215 + // CHECK-X86-64: ret i32 %[[and]] + // CHECK-PPC64: define zeroext i32 @_ZN2N54read + // CHECK-PPC64: %[[ptr:.*]] = bitcast %{{.*}}* %{{.*}} to i32* + // CHECK-PPC64: %[[val:.*]] = load i32* %[[ptr]] + // CHECK-PPC64: %[[shr:.*]] = lshr i32 %[[val]], 8 + // CHECK-PPC64: ret i32 %[[shr]] return u->y.b; } void write(U* u, unsigned x) { - // CHECK: define void @_ZN2N55write - // CHECK: %[[ptr:.*]] = bitcast %{{.*}}* %{{.*}} to i32* - // CHECK: %[[old:.*]] = load i32* %[[ptr]] - // CHECK: %[[x_and:.*]] = and i32 %{{.*}}, 16777215 - // CHECK: %[[old_and:.*]] = and i32 %[[old]], -16777216 - // CHECK: %[[new:.*]] = or i32 %[[old_and]], %[[x_and]] - // CHECK: store i32 %[[new]], i32* %[[ptr]] + // CHECK-X86-64: define void @_ZN2N55write + // CHECK-X86-64: %[[ptr:.*]] = bitcast %{{.*}}* %{{.*}} to i32* + // CHECK-X86-64: %[[old:.*]] = load i32* %[[ptr]] + // CHECK-X86-64: %[[x_and:.*]] = and i32 %{{.*}}, 16777215 + // CHECK-X86-64: %[[old_and:.*]] = and i32 %[[old]], -16777216 + // CHECK-X86-64: %[[new:.*]] = or i32 %[[old_and]], %[[x_and]] + // CHECK-X86-64: store i32 %[[new]], i32* %[[ptr]] + // CHECK-PPC64: define void @_ZN2N55write + // CHECK-PPC64: %[[ptr:.*]] = bitcast %{{.*}}* %{{.*}} to i32* + // CHECK-PPC64: %[[old:.*]] = load i32* %[[ptr]] + // CHECK-PPC64: %[[x_and:.*]] = and i32 %{{.*}}, 16777215 + // CHECK-PPC64: %[[x_shl:.*]] = shl i32 %[[x_and]], 8 + // CHECK-PPC64: %[[old_and:.*]] = and i32 %[[old]], 255 + // CHECK-PPC64: %[[new:.*]] = or i32 %[[old_and]], %[[x_shl]] + // CHECK-PPC64: store i32 %[[new]], i32* %[[ptr]] u->y.b = x; } } @@ -170,25 +240,41 @@ namespace N6 { unsigned char b2 : 8; }; unsigned read(S* s) { - // CHECK: define i32 @_ZN2N64read - // CHECK: %[[ptr1:.*]] = bitcast {{.*}}* %{{.*}} to i24* - // CHECK: %[[val1:.*]] = load i24* %[[ptr1]] - // CHECK: %[[ext1:.*]] = zext i24 %[[val1]] to i32 - // CHECK: %[[ptr2:.*]] = getelementptr inbounds {{.*}}* %{{.*}}, i32 0, i32 1 - // CHECK: %[[val2:.*]] = load i8* %[[ptr2]] - // CHECK: %[[ext2:.*]] = zext i8 %[[val2]] to i32 - // CHECK: %[[add:.*]] = add nsw i32 %[[ext1]], %[[ext2]] - // CHECK: ret i32 %[[add]] + // CHECK-X86-64: define i32 @_ZN2N64read + // CHECK-X86-64: %[[ptr1:.*]] = bitcast {{.*}}* %{{.*}} to i24* + // CHECK-X86-64: %[[val1:.*]] = load i24* %[[ptr1]] + // CHECK-X86-64: %[[ext1:.*]] = zext i24 %[[val1]] to i32 + // CHECK-X86-64: %[[ptr2:.*]] = getelementptr inbounds {{.*}}* %{{.*}}, i32 0, i32 1 + // CHECK-X86-64: %[[val2:.*]] = load i8* %[[ptr2]] + // CHECK-X86-64: %[[ext2:.*]] = zext i8 %[[val2]] to i32 + // CHECK-X86-64: %[[add:.*]] = add nsw i32 %[[ext1]], %[[ext2]] + // CHECK-X86-64: ret i32 %[[add]] + // CHECK-PPC64: define zeroext i32 @_ZN2N64read + // CHECK-PPC64: %[[ptr1:.*]] = bitcast {{.*}}* %{{.*}} to i24* + // CHECK-PPC64: %[[val1:.*]] = load i24* %[[ptr1]] + // CHECK-PPC64: %[[ext1:.*]] = zext i24 %[[val1]] to i32 + // CHECK-PPC64: %[[ptr2:.*]] = getelementptr inbounds {{.*}}* %{{.*}}, i32 0, i32 1 + // CHECK-PPC64: %[[val2:.*]] = load i8* %[[ptr2]] + // CHECK-PPC64: %[[ext2:.*]] = zext i8 %[[val2]] to i32 + // CHECK-PPC64: %[[add:.*]] = add nsw i32 %[[ext1]], %[[ext2]] + // CHECK-PPC64: ret i32 %[[add]] return s->b1 + s->b2; } void write(S* s, unsigned x) { - // CHECK: define void @_ZN2N65write - // CHECK: %[[ptr1:.*]] = bitcast {{.*}}* %{{.*}} to i24* - // CHECK: %[[new1:.*]] = trunc i32 %{{.*}} to i24 - // CHECK: store i24 %[[new1]], i24* %[[ptr1]] - // CHECK: %[[new2:.*]] = trunc i32 %{{.*}} to i8 - // CHECK: %[[ptr2:.*]] = getelementptr inbounds {{.*}}* %{{.*}}, i32 0, i32 1 - // CHECK: store i8 %[[new2]], i8* %[[ptr2]] + // CHECK-X86-64: define void @_ZN2N65write + // CHECK-X86-64: %[[ptr1:.*]] = bitcast {{.*}}* %{{.*}} to i24* + // CHECK-X86-64: %[[new1:.*]] = trunc i32 %{{.*}} to i24 + // CHECK-X86-64: store i24 %[[new1]], i24* %[[ptr1]] + // CHECK-X86-64: %[[new2:.*]] = trunc i32 %{{.*}} to i8 + // CHECK-X86-64: %[[ptr2:.*]] = getelementptr inbounds {{.*}}* %{{.*}}, i32 0, i32 1 + // CHECK-X86-64: store i8 %[[new2]], i8* %[[ptr2]] + // CHECK-PPC64: define void @_ZN2N65write + // CHECK-PPC64: %[[ptr1:.*]] = bitcast {{.*}}* %{{.*}} to i24* + // CHECK-PPC64: %[[new1:.*]] = trunc i32 %{{.*}} to i24 + // CHECK-PPC64: store i24 %[[new1]], i24* %[[ptr1]] + // CHECK-PPC64: %[[new2:.*]] = trunc i32 %{{.*}} to i8 + // CHECK-PPC64: %[[ptr2:.*]] = getelementptr inbounds {{.*}}* %{{.*}}, i32 0, i32 1 + // CHECK-PPC64: store i8 %[[new2]], i8* %[[ptr2]] s->b1 = x; s->b2 = x; }