[Clang] Fix incorrect passing of _BitInt args #90741
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-backend-aarch64 Author: None (Lukacma) ChangesThis patch removes incorrect Full diff: https://github.com/llvm/llvm-project/pull/90741.diff 7 Files Affected:
diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h
index 3ced2e7397a754..488b166a95af7d 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -518,6 +518,16 @@ class TargetInfo : public TransferrableTargetInfo,
/// getInt128Align() - Returns the alignment of Int128.
unsigned getInt128Align() const { return Int128Align; }
+ /// getBitIntAlign/Width - Return aligned size of '_BitInt' and
+ /// 'unsigned _BitInt' for this target, in bits.
+ unsigned getBitIntWidth(unsigned NumBits) const {
+ return llvm::alignTo(NumBits, getBitIntAlign(NumBits));
+ }
+ virtual unsigned getBitIntAlign(unsigned NumBits) const {
+ return std::clamp<unsigned>(llvm::PowerOf2Ceil(NumBits), getCharWidth(),
+ getLongLongAlign());
+ }
+
/// getShortAccumWidth/Align - Return the size of 'signed short _Accum' and
/// 'unsigned short _Accum' for this target, in bits.
unsigned getShortAccumWidth() const { return ShortAccumWidth; }
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index cbf4932aff9a6b..f440af50e08a49 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -2263,9 +2263,8 @@ TypeInfo ASTContext::getTypeInfoImpl(const Type *T) const {
}
case Type::BitInt: {
const auto *EIT = cast<BitIntType>(T);
- Align = std::clamp<unsigned>(llvm::PowerOf2Ceil(EIT->getNumBits()),
- getCharWidth(), Target->getLongLongAlign());
- Width = llvm::alignTo(EIT->getNumBits(), Align);
+ Align = Target->getBitIntAlign(EIT->getNumBits());
+ Width = Target->getBitIntWidth(EIT->getNumBits());
break;
}
case Type::Record:
diff --git a/clang/lib/Basic/Targets/AArch64.cpp b/clang/lib/Basic/Targets/AArch64.cpp
index c8d243a8fb7aea..cc0ae0e2856fd0 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -1473,6 +1473,11 @@ bool AArch64TargetInfo::validatePointerAuthKey(
bool AArch64TargetInfo::hasInt128Type() const { return true; }
+unsigned AArch64TargetInfo::getBitIntAlign(unsigned NumBits) const {
+ return std::clamp<unsigned>(llvm::PowerOf2Ceil(NumBits), getCharWidth(),
+ getInt128Align());
+}
+
AArch64leTargetInfo::AArch64leTargetInfo(const llvm::Triple &Triple,
const TargetOptions &Opts)
: AArch64TargetInfo(Triple, Opts) {}
@@ -1674,4 +1679,4 @@ void RenderScript64TargetInfo::getTargetDefines(const LangOptions &Opts,
MacroBuilder &Builder) const {
Builder.defineMacro("__RENDERSCRIPT__");
AArch64leTargetInfo::getTargetDefines(Opts, Builder);
-}
+}
\ No newline at end of file
diff --git a/clang/lib/Basic/Targets/AArch64.h b/clang/lib/Basic/Targets/AArch64.h
index 12fb50286f7511..be6435007a6009 100644
--- a/clang/lib/Basic/Targets/AArch64.h
+++ b/clang/lib/Basic/Targets/AArch64.h
@@ -202,6 +202,8 @@ class LLVM_LIBRARY_VISIBILITY AArch64TargetInfo : public TargetInfo {
bool hasBitIntType() const override { return true; }
bool validateTarget(DiagnosticsEngine &Diags) const override;
+
+ unsigned getBitIntAlign(unsigned NumBits) const override;
};
class LLVM_LIBRARY_VISIBILITY AArch64leTargetInfo : public AArch64TargetInfo {
diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp b/clang/lib/CodeGen/Targets/AArch64.cpp
index 4c32f510101f04..7daf416b624fa6 100644
--- a/clang/lib/CodeGen/Targets/AArch64.cpp
+++ b/clang/lib/CodeGen/Targets/AArch64.cpp
@@ -302,7 +302,7 @@ AArch64ABIInfo::classifyArgumentType(QualType Ty, bool IsVariadic,
if (const auto *EIT = Ty->getAs<BitIntType>())
if (EIT->getNumBits() > 128)
- return getNaturalAlignIndirect(Ty);
+ return getNaturalAlignIndirect(Ty, false);
return (isPromotableIntegerTypeForABI(Ty) && isDarwinPCS()
? ABIArgInfo::getExtend(Ty)
diff --git a/clang/test/CodeGen/aapcs64-align.cpp b/clang/test/CodeGen/aapcs64-align.cpp
index de231f2123b975..7a8151022852ea 100644
--- a/clang/test/CodeGen/aapcs64-align.cpp
+++ b/clang/test/CodeGen/aapcs64-align.cpp
@@ -1,7 +1,7 @@
// REQUIRES: arm-registered-target
// RUN: %clang_cc1 -triple aarch64-none-elf \
// RUN: -O2 \
-// RUN: -emit-llvm -o - %s | FileCheck %s
+// RUN: -emit-llvm -fexperimental-max-bitint-width=1024 -o - %s | FileCheck %s
extern "C" {
@@ -100,4 +100,66 @@ void f5m(int, int, int, int, int, P16);
// CHECK: declare void @f5(i32 noundef, [2 x i64])
// CHECK: declare void @f5m(i32 noundef, i32 noundef, i32 noundef, i32 noundef, i32 noundef, [2 x i64])
+//BitInt alignment
+struct BITINT129 {
+ char ch;
+ unsigned _BitInt(129) v;
+};
+
+int test_bitint129(){
+ return __builtin_offsetof(struct BITINT129, v);
}
+// CHECK: ret i32 16
+
+struct BITINT127 {
+ char ch;
+ _BitInt(127) v;
+};
+
+int test_bitint127(){
+ return __builtin_offsetof(struct BITINT127, v);
+}
+// CHECK: ret i32 16
+
+struct BITINT63 {
+ char ch;
+ _BitInt(63) v;
+};
+
+int test_bitint63(){
+ return __builtin_offsetof(struct BITINT63, v);
+}
+// CHECK: ret i32 8
+
+struct BITINT32 {
+ char ch;
+ unsigned _BitInt(32) v;
+};
+
+int test_bitint32(){
+ return __builtin_offsetof(struct BITINT32, v);
+}
+// CHECK: ret i32 4
+
+struct BITINT9 {
+ char ch;
+ unsigned _BitInt(9) v;
+};
+
+int test_bitint9(){
+ return __builtin_offsetof(struct BITINT9, v);
+}
+// CHECK: ret i32 2
+
+struct BITINT8 {
+ char ch;
+ unsigned _BitInt(8) v;
+};
+
+int test_bitint8(){
+ return __builtin_offsetof(struct BITINT8, v);
+}
+// CHECK: ret i32 1
+
+}
+
diff --git a/clang/test/CodeGen/aarch64-bitint-argpass.c b/clang/test/CodeGen/aarch64-bitint-argpass.c
new file mode 100644
index 00000000000000..c7333bac75c1ac
--- /dev/null
+++ b/clang/test/CodeGen/aarch64-bitint-argpass.c
@@ -0,0 +1,14 @@
+// REQUIRES: arm-registered-target
+// RUN: %clang_cc1 -triple aarch64-none-elf \
+// RUN: -O2 \
+// RUN: -emit-llvm -fexperimental-max-bitint-width=1024 -o - %s | FileCheck %s
+
+_BitInt(129) v = -1;
+int h(_BitInt(129));
+
+// CHECK: declare i32 @h(ptr noundef)
+int largerthan128() {
+ return h(v);
+}
+
+
|
|
@llvm/pr-subscribers-clang-codegen Author: None (Lukacma) ChangesThis patch removes incorrect Full diff: https://github.com/llvm/llvm-project/pull/90741.diff 7 Files Affected:
diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h
index 3ced2e7397a754..488b166a95af7d 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -518,6 +518,16 @@ class TargetInfo : public TransferrableTargetInfo,
/// getInt128Align() - Returns the alignment of Int128.
unsigned getInt128Align() const { return Int128Align; }
+ /// getBitIntAlign/Width - Return aligned size of '_BitInt' and
+ /// 'unsigned _BitInt' for this target, in bits.
+ unsigned getBitIntWidth(unsigned NumBits) const {
+ return llvm::alignTo(NumBits, getBitIntAlign(NumBits));
+ }
+ virtual unsigned getBitIntAlign(unsigned NumBits) const {
+ return std::clamp<unsigned>(llvm::PowerOf2Ceil(NumBits), getCharWidth(),
+ getLongLongAlign());
+ }
+
/// getShortAccumWidth/Align - Return the size of 'signed short _Accum' and
/// 'unsigned short _Accum' for this target, in bits.
unsigned getShortAccumWidth() const { return ShortAccumWidth; }
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index cbf4932aff9a6b..f440af50e08a49 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -2263,9 +2263,8 @@ TypeInfo ASTContext::getTypeInfoImpl(const Type *T) const {
}
case Type::BitInt: {
const auto *EIT = cast<BitIntType>(T);
- Align = std::clamp<unsigned>(llvm::PowerOf2Ceil(EIT->getNumBits()),
- getCharWidth(), Target->getLongLongAlign());
- Width = llvm::alignTo(EIT->getNumBits(), Align);
+ Align = Target->getBitIntAlign(EIT->getNumBits());
+ Width = Target->getBitIntWidth(EIT->getNumBits());
break;
}
case Type::Record:
diff --git a/clang/lib/Basic/Targets/AArch64.cpp b/clang/lib/Basic/Targets/AArch64.cpp
index c8d243a8fb7aea..cc0ae0e2856fd0 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -1473,6 +1473,11 @@ bool AArch64TargetInfo::validatePointerAuthKey(
bool AArch64TargetInfo::hasInt128Type() const { return true; }
+unsigned AArch64TargetInfo::getBitIntAlign(unsigned NumBits) const {
+ return std::clamp<unsigned>(llvm::PowerOf2Ceil(NumBits), getCharWidth(),
+ getInt128Align());
+}
+
AArch64leTargetInfo::AArch64leTargetInfo(const llvm::Triple &Triple,
const TargetOptions &Opts)
: AArch64TargetInfo(Triple, Opts) {}
@@ -1674,4 +1679,4 @@ void RenderScript64TargetInfo::getTargetDefines(const LangOptions &Opts,
MacroBuilder &Builder) const {
Builder.defineMacro("__RENDERSCRIPT__");
AArch64leTargetInfo::getTargetDefines(Opts, Builder);
-}
+}
\ No newline at end of file
diff --git a/clang/lib/Basic/Targets/AArch64.h b/clang/lib/Basic/Targets/AArch64.h
index 12fb50286f7511..be6435007a6009 100644
--- a/clang/lib/Basic/Targets/AArch64.h
+++ b/clang/lib/Basic/Targets/AArch64.h
@@ -202,6 +202,8 @@ class LLVM_LIBRARY_VISIBILITY AArch64TargetInfo : public TargetInfo {
bool hasBitIntType() const override { return true; }
bool validateTarget(DiagnosticsEngine &Diags) const override;
+
+ unsigned getBitIntAlign(unsigned NumBits) const override;
};
class LLVM_LIBRARY_VISIBILITY AArch64leTargetInfo : public AArch64TargetInfo {
diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp b/clang/lib/CodeGen/Targets/AArch64.cpp
index 4c32f510101f04..7daf416b624fa6 100644
--- a/clang/lib/CodeGen/Targets/AArch64.cpp
+++ b/clang/lib/CodeGen/Targets/AArch64.cpp
@@ -302,7 +302,7 @@ AArch64ABIInfo::classifyArgumentType(QualType Ty, bool IsVariadic,
if (const auto *EIT = Ty->getAs<BitIntType>())
if (EIT->getNumBits() > 128)
- return getNaturalAlignIndirect(Ty);
+ return getNaturalAlignIndirect(Ty, false);
return (isPromotableIntegerTypeForABI(Ty) && isDarwinPCS()
? ABIArgInfo::getExtend(Ty)
diff --git a/clang/test/CodeGen/aapcs64-align.cpp b/clang/test/CodeGen/aapcs64-align.cpp
index de231f2123b975..7a8151022852ea 100644
--- a/clang/test/CodeGen/aapcs64-align.cpp
+++ b/clang/test/CodeGen/aapcs64-align.cpp
@@ -1,7 +1,7 @@
// REQUIRES: arm-registered-target
// RUN: %clang_cc1 -triple aarch64-none-elf \
// RUN: -O2 \
-// RUN: -emit-llvm -o - %s | FileCheck %s
+// RUN: -emit-llvm -fexperimental-max-bitint-width=1024 -o - %s | FileCheck %s
extern "C" {
@@ -100,4 +100,66 @@ void f5m(int, int, int, int, int, P16);
// CHECK: declare void @f5(i32 noundef, [2 x i64])
// CHECK: declare void @f5m(i32 noundef, i32 noundef, i32 noundef, i32 noundef, i32 noundef, [2 x i64])
+//BitInt alignment
+struct BITINT129 {
+ char ch;
+ unsigned _BitInt(129) v;
+};
+
+int test_bitint129(){
+ return __builtin_offsetof(struct BITINT129, v);
}
+// CHECK: ret i32 16
+
+struct BITINT127 {
+ char ch;
+ _BitInt(127) v;
+};
+
+int test_bitint127(){
+ return __builtin_offsetof(struct BITINT127, v);
+}
+// CHECK: ret i32 16
+
+struct BITINT63 {
+ char ch;
+ _BitInt(63) v;
+};
+
+int test_bitint63(){
+ return __builtin_offsetof(struct BITINT63, v);
+}
+// CHECK: ret i32 8
+
+struct BITINT32 {
+ char ch;
+ unsigned _BitInt(32) v;
+};
+
+int test_bitint32(){
+ return __builtin_offsetof(struct BITINT32, v);
+}
+// CHECK: ret i32 4
+
+struct BITINT9 {
+ char ch;
+ unsigned _BitInt(9) v;
+};
+
+int test_bitint9(){
+ return __builtin_offsetof(struct BITINT9, v);
+}
+// CHECK: ret i32 2
+
+struct BITINT8 {
+ char ch;
+ unsigned _BitInt(8) v;
+};
+
+int test_bitint8(){
+ return __builtin_offsetof(struct BITINT8, v);
+}
+// CHECK: ret i32 1
+
+}
+
diff --git a/clang/test/CodeGen/aarch64-bitint-argpass.c b/clang/test/CodeGen/aarch64-bitint-argpass.c
new file mode 100644
index 00000000000000..c7333bac75c1ac
--- /dev/null
+++ b/clang/test/CodeGen/aarch64-bitint-argpass.c
@@ -0,0 +1,14 @@
+// REQUIRES: arm-registered-target
+// RUN: %clang_cc1 -triple aarch64-none-elf \
+// RUN: -O2 \
+// RUN: -emit-llvm -fexperimental-max-bitint-width=1024 -o - %s | FileCheck %s
+
+_BitInt(129) v = -1;
+int h(_BitInt(129));
+
+// CHECK: declare i32 @h(ptr noundef)
+int largerthan128() {
+ return h(v);
+}
+
+
|
|
This PR is dependent on #90602 to be merged. |
| // REQUIRES: arm-registered-target | ||
| // RUN: %clang_cc1 -triple aarch64-none-elf \ | ||
| // RUN: -O2 \ | ||
| // RUN: -emit-llvm -fexperimental-max-bitint-width=1024 -o - %s | FileCheck %s |
There was a problem hiding this comment.
Can we integrate this into some existing test file for aarch64 calling conventions?
There was a problem hiding this comment.
I don't think such test file exists for aarch64. I tried searching for smth like that in CodeGen, couldn't find any file which would be suitable for this test.
There was a problem hiding this comment.
Digging a bit into the history, I found clang/test/CodeGen/ext-int-cc.c . Which apparently has broken CHECK-NOT lines.
There was a problem hiding this comment.
Thank you for pointing me to this ! I didn't expect there would a general tests for all ABIs ! I have adjusted the test so it correctly checks for sizes >128. Probably better fix would be to correctly set BITINT_MAXWIDTH in AArch64 target, but that is story for separate patch.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch removes incorrect
byvalattribute from pointer argument passed with >128 bit long _BitInt types.