[Clang][AArch64][ARM]: Fix Inefficient loads/stores of _BitInt(N) #93495

hassnaaHamdi · 2024-05-28T01:27:08Z

Update clang codegen for loads/stores to read/write the legal in-memory representation for _BitInt(N <= 128) and _BitInt(N <= 64).
AArch64: for _BitInt(N <= 128) the machine type is the smallest (un)signed fundamental integral data types.
ARM: for _BitInt(N <= 64) the machine type is the smallest (un)signed fundamental integral data types.
So, Loads and Stores will be as following:
N - bit-precise integer size as declared
M - number of bits in the representation, M >= N

Loads
%u = load iM, ptr %p
%v = trunc iM %u to iN

Stores
%u = Xext iN %v to iM
store iM %u, ptr %p
where Xext is zext or sext on ARM, depending on C type, and zext for AArch64.

These changes are according to the ABI documentation for:
ARM: https://github.com/ARM-software/abi-aa/blob/main/aapcs32/aapcs32.rst
AArch64: https://github.com/ARM-software/abi-aa/blob/main/aapcs64/aapcs64.rst

llvmbot · 2024-05-28T01:27:37Z

@llvm/pr-subscribers-clang-codegen
@llvm/pr-subscribers-backend-aarch64

@llvm/pr-subscribers-backend-arm

Author: Hassnaa Hamdi (hassnaaHamdi)

Changes

Update clang codegen for loads/stores to read/write the legal in-memory representation for _BitInt(N <= 128) and _BitInt(N <= 64).
AArch64: for _BitInt(N <= 128) the machine type is the smallest (un)signed fundamental integral data types.
ARM: for _BitInt(N <= 64) the machine type is the smallest (un)signed fundamental integral data types.
So, Loads and Stores will be as following:
N - bit-precise integer size as declared
M - number of bits in the representation, M >= N

Loads
%u = load iM, ptr %p
%v = trunc iM %u to iN

Stores
%u = Xext iN %v to iM
store iM %u, ptr %p
where Xext is zext or sext on ARM, depending on C type, and zext for AArch64.

These changes are according to the ABI documentation for:
ARM: https://github.com/ARM-software/abi-aa/blob/main/aapcs32/aapcs32.rst
AArch64: https://github.com/ARM-software/abi-aa/blob/main/aapcs64/aapcs64.rst

Full diff: https://github.com/llvm/llvm-project/pull/93495.diff

11 Files Affected:

(modified) clang/include/clang/Basic/TargetInfo.h (+9)
(modified) clang/lib/Basic/Targets/AArch64.cpp (+10)
(modified) clang/lib/Basic/Targets/AArch64.h (+2)
(modified) clang/lib/Basic/Targets/ARM.cpp (+16)
(modified) clang/lib/Basic/Targets/ARM.h (+4)
(modified) clang/lib/CodeGen/CGExpr.cpp (+16-1)
(modified) clang/lib/CodeGen/CodeGenTypes.cpp (+3)
(added) clang/test/CodeGen/AArch64/BitInt.c (+35)
(added) clang/test/CodeGen/Arm/BitInt.c (+36)
(modified) clang/test/CodeGen/attr-noundef.cpp (+3-3)
(modified) clang/test/CodeGen/builtins-bitint.c (+30-24)

diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h
index 8a6511b9ced83..92b03e8d6bdbc 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -667,6 +667,15 @@ class TargetInfo : public TransferrableTargetInfo,
     return false;
   }
 
+  // Different targets may support different machine type width for the _BitInt
+  virtual unsigned getBitIntLegalWidth(unsigned Width) const {
+    return Width;
+  }
+
+  virtual bool isBitIntSignExtended(bool IsSigned) const {
+    return false;
+  }
+
   // Different targets may support a different maximum width for the _BitInt
   // type, depending on what operations are supported.
   virtual size_t getMaxBitIntWidth() const {
diff --git a/clang/lib/Basic/Targets/AArch64.cpp b/clang/lib/Basic/Targets/AArch64.cpp
index 5db1ce78c657f..a5d66cb44b566 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -221,6 +221,16 @@ bool AArch64TargetInfo::validateTarget(DiagnosticsEngine &Diags) const {
   return true;
 }
 
+unsigned AArch64TargetInfo::getBitIntLegalWidth(unsigned Width) const {
+  unsigned IntegralSizes[] = {32, 64, 128};
+  const unsigned ARR_SZ = sizeof(IntegralSizes) / sizeof(unsigned);
+  for (unsigned I = 0; I < ARR_SZ; I ++) {
+    if (IntegralSizes[I] > Width)
+      return IntegralSizes[I];
+  }
+  return Width;
+}
+
 bool AArch64TargetInfo::validateBranchProtection(StringRef Spec, StringRef,
                                                  BranchProtectionInfo &BPI,
                                                  StringRef &Err) const {
diff --git a/clang/lib/Basic/Targets/AArch64.h b/clang/lib/Basic/Targets/AArch64.h
index 12fb50286f751..89c6af00e628c 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 getBitIntLegalWidth(unsigned Width) const override;
 };
 
 class LLVM_LIBRARY_VISIBILITY AArch64leTargetInfo : public AArch64TargetInfo {
diff --git a/clang/lib/Basic/Targets/ARM.cpp b/clang/lib/Basic/Targets/ARM.cpp
index 7423626d7c3cb..a074bafacc25f 100644
--- a/clang/lib/Basic/Targets/ARM.cpp
+++ b/clang/lib/Basic/Targets/ARM.cpp
@@ -1344,6 +1344,22 @@ int ARMTargetInfo::getEHDataRegisterNumber(unsigned RegNo) const {
 
 bool ARMTargetInfo::hasSjLjLowering() const { return true; }
 
+unsigned ARMTargetInfo::getBitIntLegalWidth(unsigned Width) const {
+  unsigned IntegralSizes[] = {32, 64};
+  const unsigned ARR_SZ = sizeof(IntegralSizes) / sizeof(unsigned);
+  for (unsigned I = 0; I < ARR_SZ; I ++) {
+    if (IntegralSizes[I] > Width)
+      return IntegralSizes[I];
+  }
+  return Width;
+}
+
+bool ARMTargetInfo::isBitIntSignExtended(bool IsSigned) const {
+    if (IsSigned)
+      return true;
+    return false;
+}
+
 ARMleTargetInfo::ARMleTargetInfo(const llvm::Triple &Triple,
                                  const TargetOptions &Opts)
     : ARMTargetInfo(Triple, Opts) {}
diff --git a/clang/lib/Basic/Targets/ARM.h b/clang/lib/Basic/Targets/ARM.h
index df9855a52e61c..679c29e786cc9 100644
--- a/clang/lib/Basic/Targets/ARM.h
+++ b/clang/lib/Basic/Targets/ARM.h
@@ -229,6 +229,10 @@ class LLVM_LIBRARY_VISIBILITY ARMTargetInfo : public TargetInfo {
   std::pair<unsigned, unsigned> hardwareInterferenceSizes() const override {
     return std::make_pair(getTriple().isArch64Bit() ? 256 : 64, 64);
   }
+
+  unsigned getBitIntLegalWidth(unsigned Width) const override;
+
+  bool isBitIntSignExtended(bool IsSigned) const override;
 };
 
 class LLVM_LIBRARY_VISIBILITY ARMleTargetInfo : public ARMTargetInfo {
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index d6478cc6835d8..4030f0024df4d 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -1989,7 +1989,14 @@ llvm::Value *CodeGenFunction::EmitLoadOfScalar(Address Addr, bool Volatile,
     return EmitAtomicLoad(AtomicLValue, Loc).getScalarVal();
   }
 
-  llvm::LoadInst *Load = Builder.CreateLoad(Addr, Volatile);
+  llvm::LoadInst *Load = nullptr;
+  if (const auto *BitintTy = Ty->getAs<BitIntType>()) {
+    Address TempAddr(Addr.getBasePointer(), ConvertTypeForMem(Ty), Addr.getAlignment());
+    Load = Builder.CreateLoad(TempAddr, Volatile);
+  }
+  else
+    Load = Builder.CreateLoad(Addr, Volatile);
+
   if (isNontemporal) {
     llvm::MDNode *Node = llvm::MDNode::get(
         Load->getContext(), llvm::ConstantAsMetadata::get(Builder.getInt32(1)));
@@ -2021,6 +2028,12 @@ llvm::Value *CodeGenFunction::EmitToMemory(llvm::Value *Value, QualType Ty) {
     assert(Value->getType()->isIntegerTy(getContext().getTypeSize(Ty)) &&
            "wrong value rep of bool");
   }
+  if(auto *BitIntTy = Ty->getAs<BitIntType>()) {
+    if (CGM.getTarget().isBitIntSignExtended(BitIntTy->isSigned()))
+      return Builder.CreateSExt(Value, ConvertTypeForMem(Ty), "sext_bitint");
+    else
+      return Builder.CreateZExt(Value, ConvertTypeForMem(Ty), "zext_bitint");
+  }
 
   return Value;
 }
@@ -2043,6 +2056,8 @@ llvm::Value *CodeGenFunction::EmitFromMemory(llvm::Value *Value, QualType Ty) {
     unsigned ValNumElems = cast<llvm::FixedVectorType>(ValTy)->getNumElements();
     return emitBoolVecConversion(V, ValNumElems, "extractvec");
   }
+  if(Ty->getAs<BitIntType>())
+    return Builder.CreateTrunc(Value, ConvertType(Ty), "to_bitint");
 
   return Value;
 }
diff --git a/clang/lib/CodeGen/CodeGenTypes.cpp b/clang/lib/CodeGen/CodeGenTypes.cpp
index 0a926e4ac27fe..743c95e5d2a2f 100644
--- a/clang/lib/CodeGen/CodeGenTypes.cpp
+++ b/clang/lib/CodeGen/CodeGenTypes.cpp
@@ -114,6 +114,9 @@ llvm::Type *CodeGenTypes::ConvertTypeForMem(QualType T, bool ForBitField) {
     return llvm::IntegerType::get(getLLVMContext(),
                                   (unsigned)Context.getTypeSize(T));
 
+  if (T->isBitIntType())
+    return llvm::IntegerType::get(getLLVMContext(),
+      CGM.getTarget().getBitIntLegalWidth(T->getAs<BitIntType>()->getNumBits()));
   // Else, don't map it.
   return R;
 }
diff --git a/clang/test/CodeGen/AArch64/BitInt.c b/clang/test/CodeGen/AArch64/BitInt.c
new file mode 100644
index 0000000000000..1784bb461ff86
--- /dev/null
+++ b/clang/test/CodeGen/AArch64/BitInt.c
@@ -0,0 +1,35 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 5
+// RUN: %clang_cc1 -triple aarch64 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple aarch64 -S -o /dev/null %s
+
+_BitInt(18) signed_src;
+_BitInt(18) signed_dst;
+
+// CHECK-LABEL: define dso_local void @test_signed(
+// CHECK-SAME: ) #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    [[TMP0:%.*]] = load i32, ptr @signed_dst, align 4
+// CHECK-NEXT:    [[TOBITINT:%.*]] = trunc i32 [[TMP0]] to i18
+// CHECK-NEXT:    [[ZEXTBITINT:%.*]] = zext i18 [[TOBITINT]] to i32
+// CHECK-NEXT:    store i32 [[ZEXTBITINT]], ptr @signed_src, align 4
+// CHECK-NEXT:    ret void
+//
+void test_signed() {
+  signed_src = signed_dst;
+}
+
+unsigned _BitInt(18) src;
+unsigned _BitInt(18) dst;
+
+// CHECK-LABEL: define dso_local void @test_unsigned(
+// CHECK-SAME: ) #[[ATTR0]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    [[TMP0:%.*]] = load i32, ptr @dst, align 4
+// CHECK-NEXT:    [[TOBITINT:%.*]] = trunc i32 [[TMP0]] to i18
+// CHECK-NEXT:    [[ZEXTBITINT:%.*]] = zext i18 [[TOBITINT]] to i32
+// CHECK-NEXT:    store i32 [[ZEXTBITINT]], ptr @src, align 4
+// CHECK-NEXT:    ret void
+//
+void test_unsigned() {
+  src = dst;
+}
diff --git a/clang/test/CodeGen/Arm/BitInt.c b/clang/test/CodeGen/Arm/BitInt.c
new file mode 100644
index 0000000000000..a8fe5029383ea
--- /dev/null
+++ b/clang/test/CodeGen/Arm/BitInt.c
@@ -0,0 +1,36 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 4
+// RUN: %clang_cc1 -triple arm -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple arm -S -o /dev/null %s
+
+_BitInt(18) signed_src;
+_BitInt(18) signed_dst;
+
+// CHECK-LABEL: define dso_local arm_aapcscc void @test_signed(
+// CHECK-SAME: ) #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[TMP0:%.*]] = load i32, ptr @signed_dst, align 4
+// CHECK-NEXT:    [[TOBITINT:%.*]] = trunc i32 [[TMP0]] to i18
+// CHECK-NEXT:    [[SEXTBITINT:%.*]] = sext i18 [[TOBITINT]] to i32
+// CHECK-NEXT:    store i32 [[SEXTBITINT]], ptr @signed_src, align 4
+// CHECK-NEXT:    ret void
+//
+void test_signed() {
+  signed_src = signed_dst;
+}
+
+unsigned _BitInt(18) src;
+unsigned _BitInt(18) dst;
+
+// CHECK-LABEL: define dso_local arm_aapcscc void @test_unsigned(
+// CHECK-SAME: ) #[[ATTR0]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[TMP0:%.*]] = load i32, ptr @dst, align 4
+// CHECK-NEXT:    [[TOBITINT:%.*]] = trunc i32 [[TMP0]] to i18
+// CHECK-NEXT:    [[ZEXTBITINT:%.*]] = zext i18 [[TOBITINT]] to i32
+// CHECK-NEXT:    store i32 [[ZEXTBITINT]], ptr @src, align 4
+// CHECK-NEXT:    ret void
+//
+void test_unsigned() {
+  src = dst;
+}
+
diff --git a/clang/test/CodeGen/attr-noundef.cpp b/clang/test/CodeGen/attr-noundef.cpp
index e1cab091bfcbf..16fc03bd6d0be 100644
--- a/clang/test/CodeGen/attr-noundef.cpp
+++ b/clang/test/CodeGen/attr-noundef.cpp
@@ -159,9 +159,9 @@ void pass_large_BitInt(_BitInt(127) e) {
 
 // TODO: for now, ExtInt is only noundef if it is sign/zero-extended
 // CHECK-INTEL: [[DEF]] noundef signext i3 @{{.*}}ret_BitInt{{.*}}()
-// CHECK-AARCH: [[DEF]] i3 @{{.*}}ret_BitInt{{.*}}()
+// CHECK-AARCH: [[DEF]] noundef i3 @{{.*}}ret_BitInt{{.*}}()
 // CHECK-INTEL: [[DEF]] void @{{.*}}pass_BitInt{{.*}}(i3 noundef signext %
-// CHECK-AARCH: [[DEF]] void @{{.*}}pass_BitInt{{.*}}(i3 %
+// CHECK-AARCH: [[DEF]] void @{{.*}}pass_BitInt{{.*}}(i3 noundef %
 // CHECK-INTEL: [[DEF]] void @{{.*}}pass_large_BitInt{{.*}}(i64 %{{.*}}, i64 %
-// CHECK-AARCH: [[DEF]] void @{{.*}}pass_large_BitInt{{.*}}(i127 %
+// CHECK-AARCH: [[DEF]] void @{{.*}}pass_large_BitInt{{.*}}(i127 noundef %
 } // namespace check_exotic
diff --git a/clang/test/CodeGen/builtins-bitint.c b/clang/test/CodeGen/builtins-bitint.c
index 804e497128773..3d103de5542ec 100644
--- a/clang/test/CodeGen/builtins-bitint.c
+++ b/clang/test/CodeGen/builtins-bitint.c
@@ -8,10 +8,11 @@
 // CHECK-O0-LABEL: define dso_local arm_aapcscc i32 @test_popcountg_ubi1(
 // CHECK-O0-SAME: ) #[[ATTR0:[0-9]+]] {
 // CHECK-O0-NEXT:  entry:
-// CHECK-O0-NEXT:    [[A:%.*]] = alloca i1, align 1
-// CHECK-O0-NEXT:    store i1 true, ptr [[A]], align 1
-// CHECK-O0-NEXT:    [[TMP0:%.*]] = load i1, ptr [[A]], align 1
-// CHECK-O0-NEXT:    [[TMP1:%.*]] = call i1 @llvm.ctpop.i1(i1 [[TMP0]])
+// CHECK-O0-NEXT:    [[A:%.*]] = alloca i32, align 1
+// CHECK-O0-NEXT:    store i32 1, ptr [[A]], align 1
+// CHECK-O0-NEXT:    [[TMP0:%.*]] = load i32, ptr [[A]], align 1
+// CHECK-O0-NEXT:    [[TO_BITINT:%.*]] = trunc i32 [[TMP0]] to i1
+// CHECK-O0-NEXT:    [[TMP1:%.*]] = call i1 @llvm.ctpop.i1(i1 [[TO_BITINT]])
 // CHECK-O0-NEXT:    [[CAST:%.*]] = zext i1 [[TMP1]] to i32
 // CHECK-O0-NEXT:    ret i32 [[CAST]]
 //
@@ -28,10 +29,11 @@ int test_popcountg_ubi1() {
 // CHECK-O0-LABEL: define dso_local arm_aapcscc i32 @test_popcountg_ubi2(
 // CHECK-O0-SAME: ) #[[ATTR0]] {
 // CHECK-O0-NEXT:  entry:
-// CHECK-O0-NEXT:    [[A:%.*]] = alloca i2, align 1
-// CHECK-O0-NEXT:    store i2 -1, ptr [[A]], align 1
-// CHECK-O0-NEXT:    [[TMP0:%.*]] = load i2, ptr [[A]], align 1
-// CHECK-O0-NEXT:    [[TMP1:%.*]] = call i2 @llvm.ctpop.i2(i2 [[TMP0]])
+// CHECK-O0-NEXT:    [[A:%.*]] = alloca i32, align 1
+// CHECK-O0-NEXT:    store i32 3, ptr [[A]], align 1
+// CHECK-O0-NEXT:    [[TMP0:%.*]] = load i32, ptr [[A]], align 1
+// CHECK-O0-NEXT:    [[TO_BITINT:%.*]] = trunc i32 [[TMP0]] to i2
+// CHECK-O0-NEXT:    [[TMP1:%.*]] = call i2 @llvm.ctpop.i2(i2 [[TO_BITINT]])
 // CHECK-O0-NEXT:    [[CAST:%.*]] = zext i2 [[TMP1]] to i32
 // CHECK-O0-NEXT:    ret i32 [[CAST]]
 //
@@ -48,10 +50,11 @@ int test_popcountg_ubi2() {
 // CHECK-O0-LABEL: define dso_local arm_aapcscc i32 @test_ctzg_ubi1(
 // CHECK-O0-SAME: ) #[[ATTR0]] {
 // CHECK-O0-NEXT:  entry:
-// CHECK-O0-NEXT:    [[A:%.*]] = alloca i1, align 1
-// CHECK-O0-NEXT:    store i1 false, ptr [[A]], align 1
-// CHECK-O0-NEXT:    [[TMP0:%.*]] = load i1, ptr [[A]], align 1
-// CHECK-O0-NEXT:    [[TMP1:%.*]] = call i1 @llvm.cttz.i1(i1 [[TMP0]], i1 false)
+// CHECK-O0-NEXT:    [[A:%.*]] = alloca i32, align 1
+// CHECK-O0-NEXT:    store i32 0, ptr [[A]], align 1
+// CHECK-O0-NEXT:    [[TMP0:%.*]] = load i32, ptr [[A]], align 1
+// CHECK-O0-NEXT:    [[TO_BITINT:%.*]] = trunc i32 [[TMP0]] to i1
+// CHECK-O0-NEXT:    [[TMP1:%.*]] = call i1 @llvm.cttz.i1(i1 [[TO_BITINT]], i1 false)
 // CHECK-O0-NEXT:    [[CAST:%.*]] = zext i1 [[TMP1]] to i32
 // CHECK-O0-NEXT:    ret i32 [[CAST]]
 //
@@ -68,10 +71,11 @@ int test_ctzg_ubi1() {
 // CHECK-O0-LABEL: define dso_local arm_aapcscc i32 @test_ctzg_ubi2(
 // CHECK-O0-SAME: ) #[[ATTR0]] {
 // CHECK-O0-NEXT:  entry:
-// CHECK-O0-NEXT:    [[A:%.*]] = alloca i2, align 1
-// CHECK-O0-NEXT:    store i2 0, ptr [[A]], align 1
-// CHECK-O0-NEXT:    [[TMP0:%.*]] = load i2, ptr [[A]], align 1
-// CHECK-O0-NEXT:    [[TMP1:%.*]] = call i2 @llvm.cttz.i2(i2 [[TMP0]], i1 false)
+// CHECK-O0-NEXT:    [[A:%.*]] = alloca i32, align 1
+// CHECK-O0-NEXT:    store i32 0, ptr [[A]], align 1
+// CHECK-O0-NEXT:    [[TMP0:%.*]] = load i32, ptr [[A]], align 1
+// CHECK-O0-NEXT:    [[TO_BITINT:%.*]] = trunc i32 [[TMP0]] to i2
+// CHECK-O0-NEXT:    [[TMP1:%.*]] = call i2 @llvm.cttz.i2(i2 [[TO_BITINT]], i1 false)
 // CHECK-O0-NEXT:    [[CAST:%.*]] = zext i2 [[TMP1]] to i32
 // CHECK-O0-NEXT:    ret i32 [[CAST]]
 //
@@ -88,10 +92,11 @@ int test_ctzg_ubi2() {
 // CHECK-O0-LABEL: define dso_local arm_aapcscc i32 @test_clzg_ubi1(
 // CHECK-O0-SAME: ) #[[ATTR0]] {
 // CHECK-O0-NEXT:  entry:
-// CHECK-O0-NEXT:    [[A:%.*]] = alloca i1, align 1
-// CHECK-O0-NEXT:    store i1 false, ptr [[A]], align 1
-// CHECK-O0-NEXT:    [[TMP0:%.*]] = load i1, ptr [[A]], align 1
-// CHECK-O0-NEXT:    [[TMP1:%.*]] = call i1 @llvm.ctlz.i1(i1 [[TMP0]], i1 false)
+// CHECK-O0-NEXT:    [[A:%.*]] = alloca i32, align 1
+// CHECK-O0-NEXT:    store i32 0, ptr [[A]], align 1
+// CHECK-O0-NEXT:    [[TMP0:%.*]] = load i32, ptr [[A]], align 1
+// CHECK-O0-NEXT:    [[TO_BITINT:%.*]] = trunc i32 [[TMP0]] to i1
+// CHECK-O0-NEXT:    [[TMP1:%.*]] = call i1 @llvm.ctlz.i1(i1 [[TO_BITINT]], i1 false)
 // CHECK-O0-NEXT:    [[CAST:%.*]] = zext i1 [[TMP1]] to i32
 // CHECK-O0-NEXT:    ret i32 [[CAST]]
 //
@@ -108,10 +113,11 @@ int test_clzg_ubi1() {
 // CHECK-O0-LABEL: define dso_local arm_aapcscc i32 @test_clzg_ubi2(
 // CHECK-O0-SAME: ) #[[ATTR0]] {
 // CHECK-O0-NEXT:  entry:
-// CHECK-O0-NEXT:    [[A:%.*]] = alloca i2, align 1
-// CHECK-O0-NEXT:    store i2 0, ptr [[A]], align 1
-// CHECK-O0-NEXT:    [[TMP0:%.*]] = load i2, ptr [[A]], align 1
-// CHECK-O0-NEXT:    [[TMP1:%.*]] = call i2 @llvm.ctlz.i2(i2 [[TMP0]], i1 false)
+// CHECK-O0-NEXT:    [[A:%.*]] = alloca i32, align 1
+// CHECK-O0-NEXT:    store i32 0, ptr [[A]], align 1
+// CHECK-O0-NEXT:    [[TMP0:%.*]] = load i32, ptr [[A]], align 1
+// CHECK-O0-NEXT:    [[TO_BITINT:%.*]] = trunc i32 [[TMP0]] to i2
+// CHECK-O0-NEXT:    [[TMP1:%.*]] = call i2 @llvm.ctlz.i2(i2 [[TO_BITINT]], i1 false)
 // CHECK-O0-NEXT:    [[CAST:%.*]] = zext i2 [[TMP1]] to i32
 // CHECK-O0-NEXT:    ret i32 [[CAST]]
 //

llvmbot · 2024-05-28T01:27:37Z

@llvm/pr-subscribers-clang

Author: Hassnaa Hamdi (hassnaaHamdi)

Changes

Update clang codegen for loads/stores to read/write the legal in-memory representation for _BitInt(N <= 128) and _BitInt(N <= 64).
AArch64: for _BitInt(N <= 128) the machine type is the smallest (un)signed fundamental integral data types.
ARM: for _BitInt(N <= 64) the machine type is the smallest (un)signed fundamental integral data types.
So, Loads and Stores will be as following:
N - bit-precise integer size as declared
M - number of bits in the representation, M >= N

Loads
%u = load iM, ptr %p
%v = trunc iM %u to iN

Stores
%u = Xext iN %v to iM
store iM %u, ptr %p
where Xext is zext or sext on ARM, depending on C type, and zext for AArch64.

These changes are according to the ABI documentation for:
ARM: https://github.com/ARM-software/abi-aa/blob/main/aapcs32/aapcs32.rst
AArch64: https://github.com/ARM-software/abi-aa/blob/main/aapcs64/aapcs64.rst

Full diff: https://github.com/llvm/llvm-project/pull/93495.diff

11 Files Affected:

(modified) clang/include/clang/Basic/TargetInfo.h (+9)
(modified) clang/lib/Basic/Targets/AArch64.cpp (+10)
(modified) clang/lib/Basic/Targets/AArch64.h (+2)
(modified) clang/lib/Basic/Targets/ARM.cpp (+16)
(modified) clang/lib/Basic/Targets/ARM.h (+4)
(modified) clang/lib/CodeGen/CGExpr.cpp (+16-1)
(modified) clang/lib/CodeGen/CodeGenTypes.cpp (+3)
(added) clang/test/CodeGen/AArch64/BitInt.c (+35)
(added) clang/test/CodeGen/Arm/BitInt.c (+36)
(modified) clang/test/CodeGen/attr-noundef.cpp (+3-3)
(modified) clang/test/CodeGen/builtins-bitint.c (+30-24)

diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h
index 8a6511b9ced83..92b03e8d6bdbc 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -667,6 +667,15 @@ class TargetInfo : public TransferrableTargetInfo,
     return false;
   }
 
+  // Different targets may support different machine type width for the _BitInt
+  virtual unsigned getBitIntLegalWidth(unsigned Width) const {
+    return Width;
+  }
+
+  virtual bool isBitIntSignExtended(bool IsSigned) const {
+    return false;
+  }
+
   // Different targets may support a different maximum width for the _BitInt
   // type, depending on what operations are supported.
   virtual size_t getMaxBitIntWidth() const {
diff --git a/clang/lib/Basic/Targets/AArch64.cpp b/clang/lib/Basic/Targets/AArch64.cpp
index 5db1ce78c657f..a5d66cb44b566 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -221,6 +221,16 @@ bool AArch64TargetInfo::validateTarget(DiagnosticsEngine &Diags) const {
   return true;
 }
 
+unsigned AArch64TargetInfo::getBitIntLegalWidth(unsigned Width) const {
+  unsigned IntegralSizes[] = {32, 64, 128};
+  const unsigned ARR_SZ = sizeof(IntegralSizes) / sizeof(unsigned);
+  for (unsigned I = 0; I < ARR_SZ; I ++) {
+    if (IntegralSizes[I] > Width)
+      return IntegralSizes[I];
+  }
+  return Width;
+}
+
 bool AArch64TargetInfo::validateBranchProtection(StringRef Spec, StringRef,
                                                  BranchProtectionInfo &BPI,
                                                  StringRef &Err) const {
diff --git a/clang/lib/Basic/Targets/AArch64.h b/clang/lib/Basic/Targets/AArch64.h
index 12fb50286f751..89c6af00e628c 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 getBitIntLegalWidth(unsigned Width) const override;
 };
 
 class LLVM_LIBRARY_VISIBILITY AArch64leTargetInfo : public AArch64TargetInfo {
diff --git a/clang/lib/Basic/Targets/ARM.cpp b/clang/lib/Basic/Targets/ARM.cpp
index 7423626d7c3cb..a074bafacc25f 100644
--- a/clang/lib/Basic/Targets/ARM.cpp
+++ b/clang/lib/Basic/Targets/ARM.cpp
@@ -1344,6 +1344,22 @@ int ARMTargetInfo::getEHDataRegisterNumber(unsigned RegNo) const {
 
 bool ARMTargetInfo::hasSjLjLowering() const { return true; }
 
+unsigned ARMTargetInfo::getBitIntLegalWidth(unsigned Width) const {
+  unsigned IntegralSizes[] = {32, 64};
+  const unsigned ARR_SZ = sizeof(IntegralSizes) / sizeof(unsigned);
+  for (unsigned I = 0; I < ARR_SZ; I ++) {
+    if (IntegralSizes[I] > Width)
+      return IntegralSizes[I];
+  }
+  return Width;
+}
+
+bool ARMTargetInfo::isBitIntSignExtended(bool IsSigned) const {
+    if (IsSigned)
+      return true;
+    return false;
+}
+
 ARMleTargetInfo::ARMleTargetInfo(const llvm::Triple &Triple,
                                  const TargetOptions &Opts)
     : ARMTargetInfo(Triple, Opts) {}
diff --git a/clang/lib/Basic/Targets/ARM.h b/clang/lib/Basic/Targets/ARM.h
index df9855a52e61c..679c29e786cc9 100644
--- a/clang/lib/Basic/Targets/ARM.h
+++ b/clang/lib/Basic/Targets/ARM.h
@@ -229,6 +229,10 @@ class LLVM_LIBRARY_VISIBILITY ARMTargetInfo : public TargetInfo {
   std::pair<unsigned, unsigned> hardwareInterferenceSizes() const override {
     return std::make_pair(getTriple().isArch64Bit() ? 256 : 64, 64);
   }
+
+  unsigned getBitIntLegalWidth(unsigned Width) const override;
+
+  bool isBitIntSignExtended(bool IsSigned) const override;
 };
 
 class LLVM_LIBRARY_VISIBILITY ARMleTargetInfo : public ARMTargetInfo {
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index d6478cc6835d8..4030f0024df4d 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -1989,7 +1989,14 @@ llvm::Value *CodeGenFunction::EmitLoadOfScalar(Address Addr, bool Volatile,
     return EmitAtomicLoad(AtomicLValue, Loc).getScalarVal();
   }
 
-  llvm::LoadInst *Load = Builder.CreateLoad(Addr, Volatile);
+  llvm::LoadInst *Load = nullptr;
+  if (const auto *BitintTy = Ty->getAs<BitIntType>()) {
+    Address TempAddr(Addr.getBasePointer(), ConvertTypeForMem(Ty), Addr.getAlignment());
+    Load = Builder.CreateLoad(TempAddr, Volatile);
+  }
+  else
+    Load = Builder.CreateLoad(Addr, Volatile);
+
   if (isNontemporal) {
     llvm::MDNode *Node = llvm::MDNode::get(
         Load->getContext(), llvm::ConstantAsMetadata::get(Builder.getInt32(1)));
@@ -2021,6 +2028,12 @@ llvm::Value *CodeGenFunction::EmitToMemory(llvm::Value *Value, QualType Ty) {
     assert(Value->getType()->isIntegerTy(getContext().getTypeSize(Ty)) &&
            "wrong value rep of bool");
   }
+  if(auto *BitIntTy = Ty->getAs<BitIntType>()) {
+    if (CGM.getTarget().isBitIntSignExtended(BitIntTy->isSigned()))
+      return Builder.CreateSExt(Value, ConvertTypeForMem(Ty), "sext_bitint");
+    else
+      return Builder.CreateZExt(Value, ConvertTypeForMem(Ty), "zext_bitint");
+  }
 
   return Value;
 }
@@ -2043,6 +2056,8 @@ llvm::Value *CodeGenFunction::EmitFromMemory(llvm::Value *Value, QualType Ty) {
     unsigned ValNumElems = cast<llvm::FixedVectorType>(ValTy)->getNumElements();
     return emitBoolVecConversion(V, ValNumElems, "extractvec");
   }
+  if(Ty->getAs<BitIntType>())
+    return Builder.CreateTrunc(Value, ConvertType(Ty), "to_bitint");
 
   return Value;
 }
diff --git a/clang/lib/CodeGen/CodeGenTypes.cpp b/clang/lib/CodeGen/CodeGenTypes.cpp
index 0a926e4ac27fe..743c95e5d2a2f 100644
--- a/clang/lib/CodeGen/CodeGenTypes.cpp
+++ b/clang/lib/CodeGen/CodeGenTypes.cpp
@@ -114,6 +114,9 @@ llvm::Type *CodeGenTypes::ConvertTypeForMem(QualType T, bool ForBitField) {
     return llvm::IntegerType::get(getLLVMContext(),
                                   (unsigned)Context.getTypeSize(T));
 
+  if (T->isBitIntType())
+    return llvm::IntegerType::get(getLLVMContext(),
+      CGM.getTarget().getBitIntLegalWidth(T->getAs<BitIntType>()->getNumBits()));
   // Else, don't map it.
   return R;
 }
diff --git a/clang/test/CodeGen/AArch64/BitInt.c b/clang/test/CodeGen/AArch64/BitInt.c
new file mode 100644
index 0000000000000..1784bb461ff86
--- /dev/null
+++ b/clang/test/CodeGen/AArch64/BitInt.c
@@ -0,0 +1,35 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 5
+// RUN: %clang_cc1 -triple aarch64 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple aarch64 -S -o /dev/null %s
+
+_BitInt(18) signed_src;
+_BitInt(18) signed_dst;
+
+// CHECK-LABEL: define dso_local void @test_signed(
+// CHECK-SAME: ) #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    [[TMP0:%.*]] = load i32, ptr @signed_dst, align 4
+// CHECK-NEXT:    [[TOBITINT:%.*]] = trunc i32 [[TMP0]] to i18
+// CHECK-NEXT:    [[ZEXTBITINT:%.*]] = zext i18 [[TOBITINT]] to i32
+// CHECK-NEXT:    store i32 [[ZEXTBITINT]], ptr @signed_src, align 4
+// CHECK-NEXT:    ret void
+//
+void test_signed() {
+  signed_src = signed_dst;
+}
+
+unsigned _BitInt(18) src;
+unsigned _BitInt(18) dst;
+
+// CHECK-LABEL: define dso_local void @test_unsigned(
+// CHECK-SAME: ) #[[ATTR0]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    [[TMP0:%.*]] = load i32, ptr @dst, align 4
+// CHECK-NEXT:    [[TOBITINT:%.*]] = trunc i32 [[TMP0]] to i18
+// CHECK-NEXT:    [[ZEXTBITINT:%.*]] = zext i18 [[TOBITINT]] to i32
+// CHECK-NEXT:    store i32 [[ZEXTBITINT]], ptr @src, align 4
+// CHECK-NEXT:    ret void
+//
+void test_unsigned() {
+  src = dst;
+}
diff --git a/clang/test/CodeGen/Arm/BitInt.c b/clang/test/CodeGen/Arm/BitInt.c
new file mode 100644
index 0000000000000..a8fe5029383ea
--- /dev/null
+++ b/clang/test/CodeGen/Arm/BitInt.c
@@ -0,0 +1,36 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 4
+// RUN: %clang_cc1 -triple arm -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple arm -S -o /dev/null %s
+
+_BitInt(18) signed_src;
+_BitInt(18) signed_dst;
+
+// CHECK-LABEL: define dso_local arm_aapcscc void @test_signed(
+// CHECK-SAME: ) #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[TMP0:%.*]] = load i32, ptr @signed_dst, align 4
+// CHECK-NEXT:    [[TOBITINT:%.*]] = trunc i32 [[TMP0]] to i18
+// CHECK-NEXT:    [[SEXTBITINT:%.*]] = sext i18 [[TOBITINT]] to i32
+// CHECK-NEXT:    store i32 [[SEXTBITINT]], ptr @signed_src, align 4
+// CHECK-NEXT:    ret void
+//
+void test_signed() {
+  signed_src = signed_dst;
+}
+
+unsigned _BitInt(18) src;
+unsigned _BitInt(18) dst;
+
+// CHECK-LABEL: define dso_local arm_aapcscc void @test_unsigned(
+// CHECK-SAME: ) #[[ATTR0]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[TMP0:%.*]] = load i32, ptr @dst, align 4
+// CHECK-NEXT:    [[TOBITINT:%.*]] = trunc i32 [[TMP0]] to i18
+// CHECK-NEXT:    [[ZEXTBITINT:%.*]] = zext i18 [[TOBITINT]] to i32
+// CHECK-NEXT:    store i32 [[ZEXTBITINT]], ptr @src, align 4
+// CHECK-NEXT:    ret void
+//
+void test_unsigned() {
+  src = dst;
+}
+
diff --git a/clang/test/CodeGen/attr-noundef.cpp b/clang/test/CodeGen/attr-noundef.cpp
index e1cab091bfcbf..16fc03bd6d0be 100644
--- a/clang/test/CodeGen/attr-noundef.cpp
+++ b/clang/test/CodeGen/attr-noundef.cpp
@@ -159,9 +159,9 @@ void pass_large_BitInt(_BitInt(127) e) {
 
 // TODO: for now, ExtInt is only noundef if it is sign/zero-extended
 // CHECK-INTEL: [[DEF]] noundef signext i3 @{{.*}}ret_BitInt{{.*}}()
-// CHECK-AARCH: [[DEF]] i3 @{{.*}}ret_BitInt{{.*}}()
+// CHECK-AARCH: [[DEF]] noundef i3 @{{.*}}ret_BitInt{{.*}}()
 // CHECK-INTEL: [[DEF]] void @{{.*}}pass_BitInt{{.*}}(i3 noundef signext %
-// CHECK-AARCH: [[DEF]] void @{{.*}}pass_BitInt{{.*}}(i3 %
+// CHECK-AARCH: [[DEF]] void @{{.*}}pass_BitInt{{.*}}(i3 noundef %
 // CHECK-INTEL: [[DEF]] void @{{.*}}pass_large_BitInt{{.*}}(i64 %{{.*}}, i64 %
-// CHECK-AARCH: [[DEF]] void @{{.*}}pass_large_BitInt{{.*}}(i127 %
+// CHECK-AARCH: [[DEF]] void @{{.*}}pass_large_BitInt{{.*}}(i127 noundef %
 } // namespace check_exotic
diff --git a/clang/test/CodeGen/builtins-bitint.c b/clang/test/CodeGen/builtins-bitint.c
index 804e497128773..3d103de5542ec 100644
--- a/clang/test/CodeGen/builtins-bitint.c
+++ b/clang/test/CodeGen/builtins-bitint.c
@@ -8,10 +8,11 @@
 // CHECK-O0-LABEL: define dso_local arm_aapcscc i32 @test_popcountg_ubi1(
 // CHECK-O0-SAME: ) #[[ATTR0:[0-9]+]] {
 // CHECK-O0-NEXT:  entry:
-// CHECK-O0-NEXT:    [[A:%.*]] = alloca i1, align 1
-// CHECK-O0-NEXT:    store i1 true, ptr [[A]], align 1
-// CHECK-O0-NEXT:    [[TMP0:%.*]] = load i1, ptr [[A]], align 1
-// CHECK-O0-NEXT:    [[TMP1:%.*]] = call i1 @llvm.ctpop.i1(i1 [[TMP0]])
+// CHECK-O0-NEXT:    [[A:%.*]] = alloca i32, align 1
+// CHECK-O0-NEXT:    store i32 1, ptr [[A]], align 1
+// CHECK-O0-NEXT:    [[TMP0:%.*]] = load i32, ptr [[A]], align 1
+// CHECK-O0-NEXT:    [[TO_BITINT:%.*]] = trunc i32 [[TMP0]] to i1
+// CHECK-O0-NEXT:    [[TMP1:%.*]] = call i1 @llvm.ctpop.i1(i1 [[TO_BITINT]])
 // CHECK-O0-NEXT:    [[CAST:%.*]] = zext i1 [[TMP1]] to i32
 // CHECK-O0-NEXT:    ret i32 [[CAST]]
 //
@@ -28,10 +29,11 @@ int test_popcountg_ubi1() {
 // CHECK-O0-LABEL: define dso_local arm_aapcscc i32 @test_popcountg_ubi2(
 // CHECK-O0-SAME: ) #[[ATTR0]] {
 // CHECK-O0-NEXT:  entry:
-// CHECK-O0-NEXT:    [[A:%.*]] = alloca i2, align 1
-// CHECK-O0-NEXT:    store i2 -1, ptr [[A]], align 1
-// CHECK-O0-NEXT:    [[TMP0:%.*]] = load i2, ptr [[A]], align 1
-// CHECK-O0-NEXT:    [[TMP1:%.*]] = call i2 @llvm.ctpop.i2(i2 [[TMP0]])
+// CHECK-O0-NEXT:    [[A:%.*]] = alloca i32, align 1
+// CHECK-O0-NEXT:    store i32 3, ptr [[A]], align 1
+// CHECK-O0-NEXT:    [[TMP0:%.*]] = load i32, ptr [[A]], align 1
+// CHECK-O0-NEXT:    [[TO_BITINT:%.*]] = trunc i32 [[TMP0]] to i2
+// CHECK-O0-NEXT:    [[TMP1:%.*]] = call i2 @llvm.ctpop.i2(i2 [[TO_BITINT]])
 // CHECK-O0-NEXT:    [[CAST:%.*]] = zext i2 [[TMP1]] to i32
 // CHECK-O0-NEXT:    ret i32 [[CAST]]
 //
@@ -48,10 +50,11 @@ int test_popcountg_ubi2() {
 // CHECK-O0-LABEL: define dso_local arm_aapcscc i32 @test_ctzg_ubi1(
 // CHECK-O0-SAME: ) #[[ATTR0]] {
 // CHECK-O0-NEXT:  entry:
-// CHECK-O0-NEXT:    [[A:%.*]] = alloca i1, align 1
-// CHECK-O0-NEXT:    store i1 false, ptr [[A]], align 1
-// CHECK-O0-NEXT:    [[TMP0:%.*]] = load i1, ptr [[A]], align 1
-// CHECK-O0-NEXT:    [[TMP1:%.*]] = call i1 @llvm.cttz.i1(i1 [[TMP0]], i1 false)
+// CHECK-O0-NEXT:    [[A:%.*]] = alloca i32, align 1
+// CHECK-O0-NEXT:    store i32 0, ptr [[A]], align 1
+// CHECK-O0-NEXT:    [[TMP0:%.*]] = load i32, ptr [[A]], align 1
+// CHECK-O0-NEXT:    [[TO_BITINT:%.*]] = trunc i32 [[TMP0]] to i1
+// CHECK-O0-NEXT:    [[TMP1:%.*]] = call i1 @llvm.cttz.i1(i1 [[TO_BITINT]], i1 false)
 // CHECK-O0-NEXT:    [[CAST:%.*]] = zext i1 [[TMP1]] to i32
 // CHECK-O0-NEXT:    ret i32 [[CAST]]
 //
@@ -68,10 +71,11 @@ int test_ctzg_ubi1() {
 // CHECK-O0-LABEL: define dso_local arm_aapcscc i32 @test_ctzg_ubi2(
 // CHECK-O0-SAME: ) #[[ATTR0]] {
 // CHECK-O0-NEXT:  entry:
-// CHECK-O0-NEXT:    [[A:%.*]] = alloca i2, align 1
-// CHECK-O0-NEXT:    store i2 0, ptr [[A]], align 1
-// CHECK-O0-NEXT:    [[TMP0:%.*]] = load i2, ptr [[A]], align 1
-// CHECK-O0-NEXT:    [[TMP1:%.*]] = call i2 @llvm.cttz.i2(i2 [[TMP0]], i1 false)
+// CHECK-O0-NEXT:    [[A:%.*]] = alloca i32, align 1
+// CHECK-O0-NEXT:    store i32 0, ptr [[A]], align 1
+// CHECK-O0-NEXT:    [[TMP0:%.*]] = load i32, ptr [[A]], align 1
+// CHECK-O0-NEXT:    [[TO_BITINT:%.*]] = trunc i32 [[TMP0]] to i2
+// CHECK-O0-NEXT:    [[TMP1:%.*]] = call i2 @llvm.cttz.i2(i2 [[TO_BITINT]], i1 false)
 // CHECK-O0-NEXT:    [[CAST:%.*]] = zext i2 [[TMP1]] to i32
 // CHECK-O0-NEXT:    ret i32 [[CAST]]
 //
@@ -88,10 +92,11 @@ int test_ctzg_ubi2() {
 // CHECK-O0-LABEL: define dso_local arm_aapcscc i32 @test_clzg_ubi1(
 // CHECK-O0-SAME: ) #[[ATTR0]] {
 // CHECK-O0-NEXT:  entry:
-// CHECK-O0-NEXT:    [[A:%.*]] = alloca i1, align 1
-// CHECK-O0-NEXT:    store i1 false, ptr [[A]], align 1
-// CHECK-O0-NEXT:    [[TMP0:%.*]] = load i1, ptr [[A]], align 1
-// CHECK-O0-NEXT:    [[TMP1:%.*]] = call i1 @llvm.ctlz.i1(i1 [[TMP0]], i1 false)
+// CHECK-O0-NEXT:    [[A:%.*]] = alloca i32, align 1
+// CHECK-O0-NEXT:    store i32 0, ptr [[A]], align 1
+// CHECK-O0-NEXT:    [[TMP0:%.*]] = load i32, ptr [[A]], align 1
+// CHECK-O0-NEXT:    [[TO_BITINT:%.*]] = trunc i32 [[TMP0]] to i1
+// CHECK-O0-NEXT:    [[TMP1:%.*]] = call i1 @llvm.ctlz.i1(i1 [[TO_BITINT]], i1 false)
 // CHECK-O0-NEXT:    [[CAST:%.*]] = zext i1 [[TMP1]] to i32
 // CHECK-O0-NEXT:    ret i32 [[CAST]]
 //
@@ -108,10 +113,11 @@ int test_clzg_ubi1() {
 // CHECK-O0-LABEL: define dso_local arm_aapcscc i32 @test_clzg_ubi2(
 // CHECK-O0-SAME: ) #[[ATTR0]] {
 // CHECK-O0-NEXT:  entry:
-// CHECK-O0-NEXT:    [[A:%.*]] = alloca i2, align 1
-// CHECK-O0-NEXT:    store i2 0, ptr [[A]], align 1
-// CHECK-O0-NEXT:    [[TMP0:%.*]] = load i2, ptr [[A]], align 1
-// CHECK-O0-NEXT:    [[TMP1:%.*]] = call i2 @llvm.ctlz.i2(i2 [[TMP0]], i1 false)
+// CHECK-O0-NEXT:    [[A:%.*]] = alloca i32, align 1
+// CHECK-O0-NEXT:    store i32 0, ptr [[A]], align 1
+// CHECK-O0-NEXT:    [[TMP0:%.*]] = load i32, ptr [[A]], align 1
+// CHECK-O0-NEXT:    [[TO_BITINT:%.*]] = trunc i32 [[TMP0]] to i2
+// CHECK-O0-NEXT:    [[TMP1:%.*]] = call i2 @llvm.ctlz.i2(i2 [[TO_BITINT]], i1 false)
 // CHECK-O0-NEXT:    [[CAST:%.*]] = zext i2 [[TMP1]] to i32
 // CHECK-O0-NEXT:    ret i32 [[CAST]]
 //

momchil-velikov · 2024-05-28T17:15:01Z

clang/lib/Basic/Targets/AArch64.cpp

@@ -221,6 +221,16 @@ bool AArch64TargetInfo::validateTarget(DiagnosticsEngine &Diags) const {
  return true;
 }

+unsigned AArch64TargetInfo::getBitIntLegalWidth(unsigned Width) const {


This function is likely unnecessary (also it's incorrect in a couple of ways[1]). I think you should be using instead ASTConext::getTypeInfo(T).Width.

[1] Representation might be in less than 32-bits (could be also 8 or 16) and _BitInt(N), N > 128 is not N bits wide, it's in multiples of i128.

momchil-velikov · 2024-05-28T17:30:58Z

clang/lib/CodeGen/CGExpr.cpp

@@ -2021,6 +2028,12 @@ llvm::Value *CodeGenFunction::EmitToMemory(llvm::Value *Value, QualType Ty) {
    assert(Value->getType()->isIntegerTy(getContext().getTypeSize(Ty)) &&
           "wrong value rep of bool");
  }
+  if (auto *BitIntTy = Ty->getAs<BitIntType>()) {
+    if (CGM.getTarget().isBitIntSignExtended(BitIntTy->isSigned()))


We might be introducing changes that are not desirable (or correct) for non-Arm targets.
Instead, I would suggest to add a description of the in-memory padding for the _BitInt types (e.g. via a member function in TargetInfo).
One reasonable approach is lilke this:

enum class TargetBitInitPaddingKind { None, ZeroOrSignExtend, AnyExtend };

where

None will be the default and will result in identical code as the one that Clang generates now, i.e. no sext or zext, load/stores use LLVM type iN for _BitInt(N).

ZeroOrSignExtend would mean in-memory representation is padded with 0 for unsigned _BitInt(N) and with the sign bit for signed _BitInt(N). This will be the value for AArch32

AnyExtend would mean in-memory representation is padded with unspecified bits. This will be the value for AArch64. Since AFAIK we don't have such an operation in LLVM IR, one way to implement this would be identically to ZeroOrSignExtend or, alternatively, do zero-extend regardless of the signedness of the _BitInt(N) type.

efriedma-quic · 2024-05-28T18:31:36Z

How does this interact with #91364?

- Update clang codegen for loads/stores to read/write the legal in-memory representation for _BitInt(N <= 128) and _BitInt(N <= 64). - AArch64: for _BitInt(N <= 128) the machine type is the smallest (un)signed fundamental integral data types. - ARM: for _BitInt(N <= 64) the machine type is the smallest (un)signed fundamental integral data types. So, Loads and Stores will be as following: N - bit-precise integer size as declared M - number of bits in the representation, M >= N Loads %u = load iM, ptr %p %v = trunc iM %u to iN Stores %u = Xext iN %v to iM store iM %u, ptr %p where Xext is zext or sext on ARM, depending on C type, and zext for AArch64. These changes are according to the ABI documentation for: ARM: https://github.com/ARM-software/abi-aa/blob/main/aapcs32/aapcs32.rst AArch64: https://github.com/ARM-software/abi-aa/blob/main/aapcs64/aapcs64.rst Change-Id: I03d675afb4e749b00fef075aa10923682232dd79 Change-Id: I4beac3b92e06506606c8ee57866507a62ba42fba

llvmbot added clang Clang issues not falling into any other category backend:ARM backend:AArch64 clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen IR generation bugs: mangling, exceptions, etc. labels May 28, 2024

hassnaaHamdi force-pushed the bitint_ld_st branch from 0e52295 to 6b7cc14 Compare May 28, 2024 01:28

hassnaaHamdi requested review from momchil-velikov, Lukacma and CarolineConcatto May 28, 2024 10:35

momchil-velikov reviewed May 28, 2024

View reviewed changes

momchil-velikov requested review from paulwalker-arm and efriedma-quic May 28, 2024 17:31

momchil-velikov mentioned this pull request May 29, 2024

[clang] Use different memory layout type for _BitInt(N) in LLVM IR #91364

Merged

hassnaaHamdi force-pushed the bitint_ld_st branch from 6b7cc14 to e3f080d Compare May 30, 2024 18:04

hassnaaHamdi closed this Jul 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Clang][AArch64][ARM]: Fix Inefficient loads/stores of _BitInt(N) #93495

[Clang][AArch64][ARM]: Fix Inefficient loads/stores of _BitInt(N) #93495

hassnaaHamdi commented May 28, 2024

llvmbot commented May 28, 2024 •

edited

Loading

llvmbot commented May 28, 2024

momchil-velikov May 28, 2024

momchil-velikov May 28, 2024

efriedma-quic commented May 28, 2024

[Clang][AArch64][ARM]: Fix Inefficient loads/stores of _BitInt(N) #93495

[Clang][AArch64][ARM]: Fix Inefficient loads/stores of _BitInt(N) #93495

Conversation

hassnaaHamdi commented May 28, 2024

llvmbot commented May 28, 2024 • edited Loading

llvmbot commented May 28, 2024

momchil-velikov May 28, 2024

Choose a reason for hiding this comment

momchil-velikov May 28, 2024

Choose a reason for hiding this comment

efriedma-quic commented May 28, 2024

llvmbot commented May 28, 2024 •

edited

Loading