-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix tbaa.struct metadata for bitfields using big endian. #87753
Fix tbaa.struct metadata for bitfields using big endian. #87753
Conversation
@llvm/pr-subscribers-clang Author: Julian Nagele (juliannagele) ChangesWhen generating tbaa.struct metadata we treat multiple adjacent bitfields as a single "field", with one corresponding entry in the metadata. At the moment this is achieved by adding an entry for the first bitfield in the run using its StorageSize and skipping the remaining bitfields. The problem is that "first" is determined by checking that the Offset of the field in the run is 0, which breaks for big endian. Full diff: https://github.com/llvm/llvm-project/pull/87753.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/CodeGenTBAA.cpp b/clang/lib/CodeGen/CodeGenTBAA.cpp
index a1e14c5f0a8c78..0ddefc4751b08c 100644
--- a/clang/lib/CodeGen/CodeGenTBAA.cpp
+++ b/clang/lib/CodeGen/CodeGenTBAA.cpp
@@ -22,6 +22,7 @@
#include "clang/AST/Mangle.h"
#include "clang/AST/RecordLayout.h"
#include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/TargetInfo.h"
#include "llvm/ADT/SmallSet.h"
#include "llvm/IR/Constants.h"
#include "llvm/IR/LLVMContext.h"
@@ -319,7 +320,10 @@ CodeGenTBAA::CollectFields(uint64_t BaseOffset,
// base type.
if ((*i)->isBitField()) {
const CGBitFieldInfo &Info = CGRL.getBitFieldInfo(*i);
- if (Info.Offset != 0)
+ bool IsBE = Context.getTargetInfo().isBigEndian();
+ bool IsFirst = IsBE ? Info.StorageSize - (Info.Offset + Info.Size) == 0
+ : Info.Offset == 0;
+ if (!IsFirst)
continue;
unsigned CurrentBitFieldSize = Info.StorageSize;
uint64_t Size =
diff --git a/clang/test/CodeGen/tbaa-struct-bitfield-endianness.cpp b/clang/test/CodeGen/tbaa-struct-bitfield-endianness.cpp
index 80884b49ddc669..e8bb46982537bb 100644
--- a/clang/test/CodeGen/tbaa-struct-bitfield-endianness.cpp
+++ b/clang/test/CodeGen/tbaa-struct-bitfield-endianness.cpp
@@ -1,13 +1,10 @@
// RUN: %clang_cc1 -triple aarch64_be-apple-darwin -emit-llvm -o - -O1 %s | \
-// RUN: FileCheck -check-prefixes=CHECK,CHECK-BE %s
+// RUN: FileCheck -check-prefixes=CHECK %s
// RUN: %clang_cc1 -triple aarch64-apple-darwin -emit-llvm -o - -O1 %s | \
-// RUN: FileCheck -check-prefixes=CHECK,CHECK-LE %s
+// RUN: FileCheck -check-prefixes=CHECK %s
//
// Check that TBAA metadata for structs containing bitfields is
// consistent between big and little endian layouts.
-//
-// FIXME: The metadata below is invalid for the big endian layout: the
-// start offset of 2 is incorrect.
struct NamedBitfields {
int f1 : 8;
@@ -28,8 +25,7 @@ void copy(NamedBitfields *a1, NamedBitfields *a2) {
*a1 = *a2;
}
-// CHECK-BE: [[TBAA_STRUCT2]] = !{i64 2, i64 4, [[META3:![0-9]+]], i64 4, i64 4, [[META6:![0-9]+]], i64 8, i64 8, [[META8:![0-9]+]]}
-// CHECK-LE: [[TBAA_STRUCT2]] = !{i64 0, i64 4, [[META3:![0-9]+]], i64 4, i64 4, [[META6:![0-9]+]], i64 8, i64 8, [[META8:![0-9]+]]}
+// CHECK: [[TBAA_STRUCT2]] = !{i64 0, i64 4, [[META3:![0-9]+]], i64 4, i64 4, [[META6:![0-9]+]], i64 8, i64 8, [[META8:![0-9]+]]}
// CHECK: [[META3]] = !{[[META4:![0-9]+]], [[META4]], i64 0}
// CHECK: [[META4]] = !{!"omnipotent char", [[META5:![0-9]+]], i64 0}
// CHECK: [[META5]] = !{!"Simple C++ TBAA"}
|
When generating tbaa.struct metadata we treat multiple adjacent bitfields as a single "field", with one corresponding entry in the metadata. At the moment this is achieved by adding an entry for the first bitfield in the run using its StorageSize (and skipping the remaining bitfields). The problem is that "first" is determined by checking that the Offset of the field in the run is 0. This breaks for big endian.
cd73a85
to
db1ee85
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
When generating tbaa.struct metadata we treat multiple adjacent bitfields as a single "field", with one corresponding entry in the metadata. At the moment this is achieved by adding an entry for the first bitfield in the run using its StorageSize and skipping the remaining bitfields. The problem is that "first" is determined by checking that the Offset of the field in the run is 0, which breaks for big endian.