I don't think there is much need to run the test for both x86-64 and aarch64,
I'd just run the x86-64 one.
I think it's unneccesary to have two different files here, one file for both
tests is sufficient. Just rename one of the `A` classes to `B`.
Please match the existing convention in test/Layout, rename
union_regular_bit_field.cpp to include 'itanium' somewhere in there and please
use dashes instead of underscores. Perhaps itanium-x86-union-bitfield.
================
Comment at: lib/AST/RecordLayoutBuilder.cpp:1350-1351
@@ +1349,4 @@
+static uint64_t
+RoundUpFieldSizeToCharAlignment(const uint64_t FieldSize,
+ const ASTContext &Context) {
+ uint64_t CharAlignment = Context.getTargetInfo().getCharAlign();
----------------
Functions start with a lower case character.
I would just call this `roundUpSizeToCharAlignment`. This name, while more
generic, is less confusing if we chose to use it in other places of record
layout.
`const uint64_t` looks abnormal compared to most of clang's code, I would just
go with `uint64_t` here.
================
Comment at: test/Layout/union_regular_bit_field.cpp:1-4
@@ +1,5 @@
+// RUN: %clang_cc1 -emit-llvm-only -triple aarch64 -fdump-record-layouts %s
2>/dev/null \
+// RUN: | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm-only -triple x86_64 -fdump-record-layouts %s
2>/dev/null \
+// RUN: | FileCheck %s
+
----------------
These triples are not specific enough. Make sure you specify the itanium ABI
somehow.
================
Comment at: test/Layout/union_regular_bit_field.cpp:13-14
@@ +12,4 @@
+
+// CHECK: | [sizeof=4, dsize=1, align=4
+// CHECK-NEXT: | nvsize=1, nvalign=4]
+
----------------
Please include more `CHECK` lines so that more tests can easily be added to
this file.
================
Comment at: test/Layout/union_wide_bit_field.cpp:1-4
@@ +1,5 @@
+// RUN: %clang_cc1 -emit-llvm-only -triple aarch64 -fdump-record-layouts %s
2>/dev/null \
+// RUN: | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm-only -triple x86_64 -fdump-record-layouts %s
2>/dev/null \
+// RUN: | FileCheck %s
+
----------------
Likewise.
http://reviews.llvm.org/D5775
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
