[PATCH] D61225: [COFF, ARM64] Align global symbol by size for ARM64 MSVC ABI
This revision was automatically updated to reflect the committed changes.
Closed by commit rL359744: [COFF, ARM64] Align global symbol by size for ARM64
MSVC ABI (authored by TomTan, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D61225?vs=197208=197680#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61225/new/
https://reviews.llvm.org/D61225
Files:
cfe/trunk/include/clang/Basic/TargetInfo.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/Basic/Targets/AArch64.cpp
cfe/trunk/lib/Basic/Targets/AArch64.h
cfe/trunk/lib/Basic/Targets/NVPTX.cpp
cfe/trunk/test/CodeGen/arm64-microsoft-struct-align.cpp
Index: cfe/trunk/include/clang/Basic/TargetInfo.h
===
--- cfe/trunk/include/clang/Basic/TargetInfo.h
+++ cfe/trunk/include/clang/Basic/TargetInfo.h
@@ -542,7 +542,9 @@
/// getMinGlobalAlign - Return the minimum alignment of a global variable,
/// unless its alignment is explicitly reduced via attributes.
- unsigned getMinGlobalAlign() const { return MinGlobalAlign; }
+ virtual unsigned getMinGlobalAlign (uint64_t) const {
+return MinGlobalAlign;
+ }
/// Return the largest alignment for which a suitably-sized allocation with
/// '::operator new(size_t)' is guaranteed to produce a correctly-aligned
Index: cfe/trunk/test/CodeGen/arm64-microsoft-struct-align.cpp
===
--- cfe/trunk/test/CodeGen/arm64-microsoft-struct-align.cpp
+++ cfe/trunk/test/CodeGen/arm64-microsoft-struct-align.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple aarch64-windows -ffreestanding -emit-llvm -O0 \
+// RUN: -x c++ -o - %s | FileCheck %s
+
+struct size1 { char str[1]; };
+struct size2 { char str[2]; };
+struct size7 { char str[4]; };
+struct size8 { char str[8]; };
+struct size63 { char str[63]; };
+struct size64 { char str[64]; };
+
+struct size1 s1;
+// CHECK: @"?s1@@3Usize1@@A" = dso_local global %struct.size1 zeroinitializer, align 1
+
+struct size2 s2;
+// CHECK: @"?s2@@3Usize2@@A" = dso_local global %struct.size2 zeroinitializer, align 4
+
+struct size7 s7;
+// CHECK: @"?s7@@3Usize7@@A" = dso_local global %struct.size7 zeroinitializer, align 4
+
+struct size8 s8;
+// CHECK: @"?s8@@3Usize8@@A" = dso_local global %struct.size8 zeroinitializer, align 8
+
+struct size63 s63;
+// CHECK: @"?s63@@3Usize63@@A" = dso_local global %struct.size63 zeroinitializer, align 8
+
+struct size64 s64;
+// CHECK: @"?s64@@3Usize64@@A" = dso_local global %struct.size64 zeroinitializer, align 16
Index: cfe/trunk/lib/AST/ASTContext.cpp
===
--- cfe/trunk/lib/AST/ASTContext.cpp
+++ cfe/trunk/lib/AST/ASTContext.cpp
@@ -1600,8 +1600,10 @@
if (BaseT.getQualifiers().hasUnaligned())
Align = Target->getCharWidth();
if (const auto *VD = dyn_cast(D)) {
-if (VD->hasGlobalStorage() && !ForAlignof)
- Align = std::max(Align, getTargetInfo().getMinGlobalAlign());
+if (VD->hasGlobalStorage() && !ForAlignof) {
+ uint64_t TypeSize = getTypeSize(T.getTypePtr());
+ Align = std::max(Align, getTargetInfo().getMinGlobalAlign(TypeSize));
+}
}
}
@@ -2239,7 +2241,8 @@
/// getAlignOfGlobalVar - Return the alignment in bits that should be given
/// to a global variable of the specified type.
unsigned ASTContext::getAlignOfGlobalVar(QualType T) const {
- return std::max(getTypeAlign(T), getTargetInfo().getMinGlobalAlign());
+ uint64_t TypeSize = getTypeSize(T.getTypePtr());
+ return std::max(getTypeAlign(T), getTargetInfo().getMinGlobalAlign(TypeSize));
}
/// getAlignOfGlobalVarInChars - Return the alignment in characters that
Index: cfe/trunk/lib/Basic/Targets/AArch64.cpp
===
--- cfe/trunk/lib/Basic/Targets/AArch64.cpp
+++ cfe/trunk/lib/Basic/Targets/AArch64.cpp
@@ -551,6 +551,23 @@
return CCK_MicrosoftWin64;
}
+unsigned MicrosoftARM64TargetInfo::getMinGlobalAlign(uint64_t TypeSize) const {
+ unsigned Align = WindowsARM64TargetInfo::getMinGlobalAlign(TypeSize);
+
+ // MSVC does size based alignment for arm64 based on alignment section in
+ // below document, replicate that to keep alignment consistent with object
+ // files compiled by MSVC.
+ // https://docs.microsoft.com/en-us/cpp/build/arm64-windows-abi-conventions
+ if (TypeSize >= 512) { // TypeSize >= 64 bytes
+Align = std::max(Align, 128u);// align type at least 16 bytes
+ } else if (TypeSize >= 64) {// TypeSize >= 8 bytes
+Align = std::max(Align, 64u); // align type at least 8 butes
+ } else if (TypeSize >= 16) {// TypeSize >= 2 bytes
+Align = std::max(Align, 32u); // align type at least 4 bytes
+ }
+ return Align;
+}
+
[PATCH] D61225: [COFF, ARM64] Align global symbol by size for ARM64 MSVC ABI
efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61225/new/ https://reviews.llvm.org/D61225 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61225: [COFF, ARM64] Align global symbol by size for ARM64 MSVC ABI
TomTan updated this revision to Diff 197208.
TomTan added a comment.
Herald added a subscriber: jholewinski.
Added test cases and also merged this alignment adjustment to
`getMinGlobalAlign` in `MicrosoftARM64TargetInfo`.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61225/new/
https://reviews.llvm.org/D61225
Files:
include/clang/Basic/TargetInfo.h
lib/AST/ASTContext.cpp
lib/Basic/Targets/AArch64.cpp
lib/Basic/Targets/AArch64.h
lib/Basic/Targets/NVPTX.cpp
test/CodeGen/arm64-microsoft-struct-align.cpp
Index: test/CodeGen/arm64-microsoft-struct-align.cpp
===
--- test/CodeGen/arm64-microsoft-struct-align.cpp
+++ test/CodeGen/arm64-microsoft-struct-align.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple aarch64-windows -ffreestanding -emit-llvm -O0 \
+// RUN: -x c++ -o - %s | FileCheck %s
+
+struct size1 { char str[1]; };
+struct size2 { char str[2]; };
+struct size7 { char str[4]; };
+struct size8 { char str[8]; };
+struct size63 { char str[63]; };
+struct size64 { char str[64]; };
+
+struct size1 s1;
+// CHECK: @"?s1@@3Usize1@@A" = dso_local global %struct.size1 zeroinitializer, align 1
+
+struct size2 s2;
+// CHECK: @"?s2@@3Usize2@@A" = dso_local global %struct.size2 zeroinitializer, align 4
+
+struct size7 s7;
+// CHECK: @"?s7@@3Usize7@@A" = dso_local global %struct.size7 zeroinitializer, align 4
+
+struct size8 s8;
+// CHECK: @"?s8@@3Usize8@@A" = dso_local global %struct.size8 zeroinitializer, align 8
+
+struct size63 s63;
+// CHECK: @"?s63@@3Usize63@@A" = dso_local global %struct.size63 zeroinitializer, align 8
+
+struct size64 s64;
+// CHECK: @"?s64@@3Usize64@@A" = dso_local global %struct.size64 zeroinitializer, align 16
Index: lib/Basic/Targets/NVPTX.cpp
===
--- lib/Basic/Targets/NVPTX.cpp
+++ lib/Basic/Targets/NVPTX.cpp
@@ -119,7 +119,7 @@
LongAlign = HostTarget->getLongAlign();
LongLongWidth = HostTarget->getLongLongWidth();
LongLongAlign = HostTarget->getLongLongAlign();
- MinGlobalAlign = HostTarget->getMinGlobalAlign();
+ MinGlobalAlign = HostTarget->getMinGlobalAlign(/* TypeSize = */ 0);
NewAlign = HostTarget->getNewAlign();
DefaultAlignForAttributeAligned =
HostTarget->getDefaultAlignForAttributeAligned();
Index: lib/Basic/Targets/AArch64.h
===
--- lib/Basic/Targets/AArch64.h
+++ lib/Basic/Targets/AArch64.h
@@ -128,6 +128,8 @@
MacroBuilder ) const override;
TargetInfo::CallingConvKind
getCallingConvKind(bool ClangABICompat4) const override;
+
+ unsigned getMinGlobalAlign(uint64_t TypeSize) const override;
};
// ARM64 MinGW target
Index: lib/Basic/Targets/AArch64.cpp
===
--- lib/Basic/Targets/AArch64.cpp
+++ lib/Basic/Targets/AArch64.cpp
@@ -545,6 +545,23 @@
return CCK_MicrosoftWin64;
}
+unsigned MicrosoftARM64TargetInfo::getMinGlobalAlign(uint64_t TypeSize) const {
+ unsigned Align = WindowsARM64TargetInfo::getMinGlobalAlign(TypeSize);
+
+ // MSVC does size based alignment for arm64 based on alignment section in
+ // below document, replicate that to keep alignment consistent with object
+ // files compiled by MSVC.
+ // https://docs.microsoft.com/en-us/cpp/build/arm64-windows-abi-conventions
+ if (TypeSize >= 512) { // TypeSize >= 64 bytes
+Align = std::max(Align, 128u);// align type at least 16 bytes
+ } else if (TypeSize >= 64) {// TypeSize >= 8 bytes
+Align = std::max(Align, 64u); // align type at least 8 butes
+ } else if (TypeSize >= 16) {// TypeSize >= 2 bytes
+Align = std::max(Align, 32u); // align type at least 4 bytes
+ }
+ return Align;
+}
+
MinGWARM64TargetInfo::MinGWARM64TargetInfo(const llvm::Triple ,
const TargetOptions )
: WindowsARM64TargetInfo(Triple, Opts) {
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -1600,8 +1600,10 @@
if (BaseT.getQualifiers().hasUnaligned())
Align = Target->getCharWidth();
if (const auto *VD = dyn_cast(D)) {
-if (VD->hasGlobalStorage() && !ForAlignof)
- Align = std::max(Align, getTargetInfo().getMinGlobalAlign());
+if (VD->hasGlobalStorage() && !ForAlignof) {
+ uint64_t TypeSize = getTypeSize(T.getTypePtr());
+ Align = std::max(Align, getTargetInfo().getMinGlobalAlign(TypeSize));
+}
}
}
@@ -2239,7 +2241,8 @@
/// getAlignOfGlobalVar - Return the alignment in bits that should be given
/// to a global variable of the specified type.
unsigned ASTContext::getAlignOfGlobalVar(QualType T) const {
- return std::max(getTypeAlign(T), getTargetInfo().getMinGlobalAlign());
+ uint64_t
[PATCH] D61225: [COFF, ARM64] Align global symbol by size for ARM64 MSVC ABI
riccibruno added a comment. Since this is target-specific, is it right to put this here ? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61225/new/ https://reviews.llvm.org/D61225 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61225: [COFF, ARM64] Align global symbol by size for ARM64 MSVC ABI
efriedma added subscribers: mgrang, rsmith. efriedma added a comment. I was going to ask if local variables are also supposed to be aligned this way... but I guess there's no way for standard C++ code to tell without explicitly making weird alignment assumptions, so let's not worry about that. This should do the right thing, as far as I can tell. It probably would make sense to refactor the arm64-specific code into a TargetInfo API... or at least the triple check. Not sure what that would look like; maybe `virtual uint64_t getMinGlobalAlign(uint64_t TypeSize)`? Or maybe should just be "bool useMicrosoftGlobalAlign()" and keep the main logic here, if the rule is the same for multiple Windows targets. Needs a testcase in test/CodeGenCXX/ demonstrating the alignment of the emitted global in various cases. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61225/new/ https://reviews.llvm.org/D61225 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61225: [COFF, ARM64] Align global symbol by size for ARM64 MSVC ABI
TomTan created this revision.
TomTan added a reviewer: efriedma.
Herald added subscribers: cfe-commits, kristof.beyls, javed.absar.
Herald added a project: clang.
According to alignment section in below ARM64 ABI document, MSVC could increase
alignment of global data based on its total size. Clang doesn't do this.
Compile the same symbol into different alignments by Clang and MSVC could cause
link error because some instruction encodings, like 64-bit LDR/STR with
immediate, require the target to be 8 bytes aligned, and linker could choose
code stream with such LDR/STR instruction from MSVC and 4 bytes aligned data
from Clang into final image, which actually cannot be linked together (see
https://bugs.llvm.org/show_bug.cgi?id=41506 for more details).
https://docs.microsoft.com/en-us/cpp/build/arm64-windows-abi-conventions?view=vs-2019#alignment
Repository:
rC Clang
https://reviews.llvm.org/D61225
Files:
lib/AST/ASTContext.cpp
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -1600,8 +1600,25 @@
if (BaseT.getQualifiers().hasUnaligned())
Align = Target->getCharWidth();
if (const auto *VD = dyn_cast(D)) {
-if (VD->hasGlobalStorage() && !ForAlignof)
+if (VD->hasGlobalStorage() && !ForAlignof) {
Align = std::max(Align, getTargetInfo().getMinGlobalAlign());
+
+ // MSVC does size based alignment for arm64 based on alignment
section
+ // in below document, replicate that to keep alignment consistent
with
+ // object files compiled by MSVC.
+ //
https://docs.microsoft.com/en-us/cpp/build/arm64-windows-abi-conventions
+ if (getTargetInfo().getTriple().getArch() == llvm::Triple::aarch64 &&
+ getTargetInfo().getTriple().getEnvironment() ==
llvm::Triple::MSVC) {
+uint64_t TypeSize = getTypeSize(T.getTypePtr());
+if (TypeSize >= 512) { // TypeSize >= 64 bytes
+ Align = std::max(Align, 128u);// align type at least 16 bytes
+} else if (TypeSize >= 64) {// TypeSize >= 8 bytes
+ Align = std::max(Align, 64u); // align type at least 8 butes
+} else if (TypeSize >= 16) {// TypeSize >= 2 bytes
+ Align = std::max(Align, 32u); // align type at least 4 bytes
+}
+ }
+}
}
}
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -1600,8 +1600,25 @@
if (BaseT.getQualifiers().hasUnaligned())
Align = Target->getCharWidth();
if (const auto *VD = dyn_cast(D)) {
-if (VD->hasGlobalStorage() && !ForAlignof)
+if (VD->hasGlobalStorage() && !ForAlignof) {
Align = std::max(Align, getTargetInfo().getMinGlobalAlign());
+
+ // MSVC does size based alignment for arm64 based on alignment section
+ // in below document, replicate that to keep alignment consistent with
+ // object files compiled by MSVC.
+ // https://docs.microsoft.com/en-us/cpp/build/arm64-windows-abi-conventions
+ if (getTargetInfo().getTriple().getArch() == llvm::Triple::aarch64 &&
+ getTargetInfo().getTriple().getEnvironment() == llvm::Triple::MSVC) {
+uint64_t TypeSize = getTypeSize(T.getTypePtr());
+if (TypeSize >= 512) { // TypeSize >= 64 bytes
+ Align = std::max(Align, 128u);// align type at least 16 bytes
+} else if (TypeSize >= 64) {// TypeSize >= 8 bytes
+ Align = std::max(Align, 64u); // align type at least 8 butes
+} else if (TypeSize >= 16) {// TypeSize >= 2 bytes
+ Align = std::max(Align, 32u); // align type at least 4 bytes
+}
+ }
+}
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
