[clang] [llvm] [mlir] [AMDGPU] fix alignment of i128 (PR #213523)
via cfe-commits
cfe-commits at lists.llvm.org
Sun Aug 2 03:17:16 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-amdgpu
Author: Simeon David Schaub (simeonschaub)
<details>
<summary>Changes</summary>
We were observing miscompiles when using i128 in AMDGPU.jl (ref https://github.com/JuliaGPU/AMDGPU.jl/issues/1002#issuecomment-5156450276). Fix this by adding an appropriate entry to the data layout.
Assisted-by: Claude Code (claude-opus-5)
---
Full diff: https://github.com/llvm/llvm-project/pull/213523.diff
7 Files Affected:
- (modified) clang/test/CodeGen/target-data.c (+2-2)
- (modified) clang/test/CodeGenOpenCL/amdgpu-env-amdgcn.cl (+1-1)
- (modified) llvm/lib/IR/AutoUpgrade.cpp (+6)
- (modified) llvm/lib/TargetParser/TargetDataLayout.cpp (+8-2)
- (added) llvm/test/CodeGen/AMDGPU/kernarg-i128-alignment.ll (+82)
- (modified) llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp (+23-11)
- (modified) mlir/test/Conversion/GPUToROCDL/gpu-to-rocdl.mlir (+1-1)
``````````diff
diff --git a/clang/test/CodeGen/target-data.c b/clang/test/CodeGen/target-data.c
index f2a09a40ee685..3bc58aa46ce69 100644
--- a/clang/test/CodeGen/target-data.c
+++ b/clang/test/CodeGen/target-data.c
@@ -160,12 +160,12 @@
// RUN: %clang_cc1 -triple amdgpu7.01-unknown -o - -emit-llvm %s \
// RUN: | FileCheck %s -check-prefix=R600SI
-// R600SI: target datalayout = "e-m:e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-p7:160:256:256:32-p8:128:128:128:48-p9:192:256:256:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1-ni:7:8:9"
+// R600SI: target datalayout = "e-m:e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-p7:160:256:256:32-p8:128:128:128:48-p9:192:256:256:32-i64:64-i128:128-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1-ni:7:8:9"
// Test default -target-cpu
// RUN: %clang_cc1 -triple amdgpu-unknown -o - -emit-llvm %s \
// RUN: | FileCheck %s -check-prefix=R600SIDefault
-// R600SIDefault: target datalayout = "e-m:e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-p7:160:256:256:32-p8:128:128:128:48-p9:192:256:256:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1-ni:7:8:9"
+// R600SIDefault: target datalayout = "e-m:e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-p7:160:256:256:32-p8:128:128:128:48-p9:192:256:256:32-i64:64-i128:128-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1-ni:7:8:9"
// RUN: %clang_cc1 -triple arm64-unknown -o - -emit-llvm %s | \
// RUN: FileCheck %s -check-prefix=AARCH64
diff --git a/clang/test/CodeGenOpenCL/amdgpu-env-amdgcn.cl b/clang/test/CodeGenOpenCL/amdgpu-env-amdgcn.cl
index 858a7db574f38..c39d22c16606d 100644
--- a/clang/test/CodeGenOpenCL/amdgpu-env-amdgcn.cl
+++ b/clang/test/CodeGenOpenCL/amdgpu-env-amdgcn.cl
@@ -1,5 +1,5 @@
// RUN: %clang_cc1 %s -O0 -triple amdgpu -emit-llvm -o - | FileCheck %s
// RUN: %clang_cc1 %s -O0 -triple amdgpu---opencl -emit-llvm -o - | FileCheck %s
-// CHECK: target datalayout = "e-m:e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-p7:160:256:256:32-p8:128:128:128:48-p9:192:256:256:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1-ni:7:8:9"
+// CHECK: target datalayout = "e-m:e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-p7:160:256:256:32-p8:128:128:128:48-p9:192:256:256:32-i64:64-i128:128-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1-ni:7:8:9"
void foo(void) {}
diff --git a/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp
index 4502759417c5a..d0cb983fe8ed9 100644
--- a/llvm/lib/IR/AutoUpgrade.cpp
+++ b/llvm/lib/IR/AutoUpgrade.cpp
@@ -7163,6 +7163,12 @@ std::string llvm::UpgradeDataLayoutString(StringRef DL, StringRef TT) {
Res.replace(Res.find(OldP8), OldP8.size(), "-p8:128:128:128:48-");
if (!DL.contains("-p9") && !DL.starts_with("p9"))
Res.append("-p9:192:256:256:32");
+
+ // Add the alignment of i128, which used to be inherited from the i64
+ // entry. Must come after the address space upgrades above, which rely on
+ // matching against the tail of the string.
+ if (!DL.contains("-i128") && !DL.starts_with("i128"))
+ Res.append("-i128:128");
}
// Upgrade the ELF mangling mode.
diff --git a/llvm/lib/TargetParser/TargetDataLayout.cpp b/llvm/lib/TargetParser/TargetDataLayout.cpp
index 8b6f46642e4fa..1e74aa68cce16 100644
--- a/llvm/lib/TargetParser/TargetDataLayout.cpp
+++ b/llvm/lib/TargetParser/TargetDataLayout.cpp
@@ -273,10 +273,16 @@ static std::string computeAMDDataLayout(const Triple &TT) {
// (address space 7), and 128-bit non-integral buffer resourcees (address
// space 8) which cannot be non-trivilally accessed by LLVM memory operations
// like getelementptr.
+ //
+ // i128 is aligned to 16 bytes to match the ABI implemented by Clang, whose
+ // AMDGPUTargetInfo leaves Int128Align at its 128-bit default. Without an
+ // explicit entry the alignment would be inherited from i64:64, and any
+ // frontend that lays out aggregates itself would disagree with the layout
+ // LLVM computes for the corresponding LLVM struct type.
return "e-m:e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32"
"-p7:160:256:256:32-p8:128:128:128:48-p9:192:256:256:32-i64:64-"
- "v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-"
- "v1024:1024-v2048:2048-n32:64-S32-A5-G1-ni:7:8:9";
+ "i128:128-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-"
+ "v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1-ni:7:8:9";
}
static std::string computeRISCVDataLayout(const Triple &TT, StringRef ABIName) {
diff --git a/llvm/test/CodeGen/AMDGPU/kernarg-i128-alignment.ll b/llvm/test/CodeGen/AMDGPU/kernarg-i128-alignment.ll
new file mode 100644
index 0000000000000..32e4dcb90e329
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/kernarg-i128-alignment.ll
@@ -0,0 +1,82 @@
+; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx1100 < %s | FileCheck %s
+
+; The AMDGPU data layout has to give i128 an ABI alignment of 16, matching the
+; ABI implemented by Clang, whose AMDGPUTargetInfo leaves Int128Align at its
+; 128-bit default. Without an explicit entry the alignment would be inherited
+; from the i64:64 entry, and the layout LLVM computes for an aggregate would
+; then disagree with the one a frontend used when it emitted the field offsets.
+;
+; For kernel arguments that disagreement is an ABI break rather than a missed
+; optimization: the kernarg slot is sized from the data layout, so the tail of
+; the argument is never copied into the kernarg segment and loads of it run off
+; the end of the segment.
+
+; struct S { i64 a; i128 b; }: ABI align 16, size 32, offsetof(b) == 16.
+; The byref slot must be 32 bytes, not 24, and `b` must be loaded from 0x20
+; (kernarg base 16 plus a field offset of 16) which stays inside the segment.
+; CHECK-LABEL: {{^}}kernarg_i128:
+; CHECK: s_load_b128 s[{{[0-9]+:[0-9]+}}], s[0:1], 0x20
+; CHECK: .amdhsa_kernarg_size 48
+
+; An i128 following a smaller member is padded out to offset 16 rather than
+; packed at offset 8.
+; CHECK-LABEL: {{^}}kernarg_i128_after_i8:
+; CHECK: s_load_b128 s[{{[0-9]+:[0-9]+}}], s[0:1], 0x20
+; CHECK: .amdhsa_kernarg_size 48
+
+; A bare i128 kernel argument is 16-byte aligned in the kernarg segment, so it
+; starts at 16 (not 8) and the argument after it at 32 (not 24).
+; CHECK-LABEL: {{^}}kernarg_i128_scalar:
+; CHECK: s_load_b128 s[{{[0-9]+:[0-9]+}}], s[0:1], 0x10
+; CHECK: .amdhsa_kernarg_size 40
+
+; The kernel metadata is emitted once, after every function, so the per-kernel
+; argument offsets are checked here in order rather than under each label.
+; CHECK: .amdgpu_metadata
+
+; CHECK: .name: s
+; CHECK-NEXT: .offset: 16
+; CHECK-NEXT: .size: 32
+; CHECK: .kernarg_segment_size: 48
+; CHECK: .name: kernarg_i128
+;
+; CHECK: .name: s
+; CHECK-NEXT: .offset: 16
+; CHECK-NEXT: .size: 32
+; CHECK: .kernarg_segment_size: 48
+; CHECK: .name: kernarg_i128_after_i8
+;
+; CHECK: .name: a
+; CHECK-NEXT: .offset: 16
+; CHECK-NEXT: .size: 16
+; CHECK: .name: b
+; CHECK-NEXT: .offset: 32
+; CHECK-NEXT: .size: 8
+; CHECK: .kernarg_segment_size: 40
+; CHECK: .name: kernarg_i128_scalar
+
+define amdgpu_kernel void @kernarg_i128(ptr addrspace(1) %out,
+ ptr addrspace(4) byref({ i64, i128 }) align 16 %s) {
+ %pb = getelementptr inbounds i8, ptr addrspace(4) %s, i64 16
+ %b = load i128, ptr addrspace(4) %pb, align 16
+ store i128 %b, ptr addrspace(1) %out, align 16
+ ret void
+}
+
+define amdgpu_kernel void @kernarg_i128_after_i8(ptr addrspace(1) %out,
+ ptr addrspace(4) byref({ i8, i128 }) align 16 %s) {
+ %pb = getelementptr inbounds i8, ptr addrspace(4) %s, i64 16
+ %b = load i128, ptr addrspace(4) %pb, align 16
+ store i128 %b, ptr addrspace(1) %out, align 16
+ ret void
+}
+
+define amdgpu_kernel void @kernarg_i128_scalar(ptr addrspace(1) %out, i128 %a, i64 %b) {
+ %ext = zext i64 %b to i128
+ %sum = add i128 %a, %ext
+ store i128 %sum, ptr addrspace(1) %out, align 16
+ ret void
+}
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"amdhsa_code_object_version", i32 500}
diff --git a/llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp b/llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp
index a082adbf6565e..49c5055076a59 100644
--- a/llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp
+++ b/llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp
@@ -43,18 +43,26 @@ TEST(DataLayoutUpgradeTest, ValidDataLayoutUpgrade) {
// and that ANDGCN adds p7 and p8 as well.
EXPECT_EQ(UpgradeDataLayoutString("e-p:64:64", "amdgcn"),
"m:e-e-p:64:64-G1-ni:7:8:9-p7:160:256:256:32-p8:128:128:128:48-p9:"
- "192:256:256:32");
+ "192:256:256:32-i128:128");
EXPECT_EQ(UpgradeDataLayoutString("e-p:64:64-G1", "amdgcn"),
"m:e-e-p:64:64-G1-ni:7:8:9-p7:160:256:256:32-p8:128:128:128:48-p9:"
- "192:256:256:32");
+ "192:256:256:32-i128:128");
// Check that the old AMDGCN p8:128:128 definition is upgraded
EXPECT_EQ(UpgradeDataLayoutString("e-p:64:64-p8:128:128-G1", "amdgcn"),
"m:e-e-p:64:64-p8:128:128:128:48-G1-ni:7:8:9-p7:160:256:256:32-p9:"
- "192:256:256:32");
+ "192:256:256:32-i128:128");
// but that r600 does not.
EXPECT_EQ(UpgradeDataLayoutString("e-p:32:32-G1", "r600"),
"m:e-e-p:32:32-G1");
+ // Check that AMDGCN targets don't add an already declared i128 alignment,
+ // and that r600 never gains one.
+ EXPECT_EQ(UpgradeDataLayoutString("e-p:64:64-i128:64-G1", "amdgcn"),
+ "m:e-e-p:64:64-i128:64-G1-ni:7:8:9-p7:160:256:256:32-p8:128:128:128:"
+ "48-p9:192:256:256:32");
+ EXPECT_EQ(UpgradeDataLayoutString("e-p:32:32-i64:64-G1", "r600"),
+ "m:e-e-p:32:32-i64:64-G1");
+
// Ensure that the non-integral direction for address space 8 doesn't get
// added in to pointer declarations.
EXPECT_EQ(
@@ -66,7 +74,7 @@ TEST(DataLayoutUpgradeTest, ValidDataLayoutUpgrade) {
"m:e-e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-i64:"
"64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:"
"1024-v2048:2048-n32:64-S32-A5-G1-ni:7:8:9-p7:160:256:256:32-p8:128:128:"
- "128:48-p9:192:256:256:32");
+ "128:48-p9:192:256:256:32-i128:128");
// Check that SystemZ adds -S64 if needed.
EXPECT_EQ(UpgradeDataLayoutString(
@@ -158,24 +166,27 @@ TEST(DataLayoutUpgradeTest, NoDataLayoutUpgrade) {
EXPECT_EQ(UpgradeDataLayoutString("G2", "r600"), "m:e-G2");
EXPECT_EQ(UpgradeDataLayoutString("e-p:64:64-G2", "amdgcn"),
"m:e-e-p:64:64-G2-ni:7:8:9-p7:160:256:256:32-p8:128:128:128:48-p9:"
- "192:256:256:32");
+ "192:256:256:32-i128:128");
EXPECT_EQ(UpgradeDataLayoutString("G2-e-p:64:64", "amdgcn"),
"m:e-G2-e-p:64:64-ni:7:8:9-p7:160:256:256:32-p8:128:128:128:48-p9:"
- "192:256:256:32");
+ "192:256:256:32-i128:128");
EXPECT_EQ(UpgradeDataLayoutString("e-p:64:64-G0", "amdgcn"),
"m:e-e-p:64:64-G0-ni:7:8:9-p7:160:256:256:32-p8:128:128:128:48-p9:"
- "192:256:256:32");
+ "192:256:256:32-i128:128");
// Check that AMDGCN targets don't add already declared address space 7.
EXPECT_EQ(
UpgradeDataLayoutString("e-p:64:64-p7:64:64", "amdgcn"),
- "m:e-e-p:64:64-p7:64:64-G1-ni:7:8:9-p8:128:128:128:48-p9:192:256:256:32");
+ "m:e-e-p:64:64-p7:64:64-G1-ni:7:8:9-p8:128:128:128:48-p9:192:256:256:32-"
+ "i128:128");
EXPECT_EQ(
UpgradeDataLayoutString("p7:64:64-G2-e-p:64:64", "amdgcn"),
- "m:e-p7:64:64-G2-e-p:64:64-ni:7:8:9-p8:128:128:128:48-p9:192:256:256:32");
+ "m:e-p7:64:64-G2-e-p:64:64-ni:7:8:9-p8:128:128:128:48-p9:192:256:256:32-"
+ "i128:128");
EXPECT_EQ(
UpgradeDataLayoutString("e-p:64:64-p7:64:64-G1", "amdgcn"),
- "m:e-e-p:64:64-p7:64:64-G1-ni:7:8:9-p8:128:128:128:48-p9:192:256:256:32");
+ "m:e-e-p:64:64-p7:64:64-G1-ni:7:8:9-p8:128:128:128:48-p9:192:256:256:32-"
+ "i128:128");
// Check that SPIR & SPIRV targets don't add -G1 if there is already a -G
// flag.
@@ -218,7 +229,8 @@ TEST(DataLayoutUpgradeTest, EmptyDataLayout) {
EXPECT_EQ(UpgradeDataLayoutString("", "r600"), "m:e-G1");
EXPECT_EQ(
UpgradeDataLayoutString("", "amdgcn"),
- "m:e-G1-ni:7:8:9-p7:160:256:256:32-p8:128:128:128:48-p9:192:256:256:32");
+ "m:e-G1-ni:7:8:9-p7:160:256:256:32-p8:128:128:128:48-p9:192:256:256:32-"
+ "i128:128");
// Check that SPIR & SPIRV targets add G1 if it's not present.
EXPECT_EQ(UpgradeDataLayoutString("", "spir"), "G1");
diff --git a/mlir/test/Conversion/GPUToROCDL/gpu-to-rocdl.mlir b/mlir/test/Conversion/GPUToROCDL/gpu-to-rocdl.mlir
index 68a5328b8eb77..396f9bb63d84f 100755
--- a/mlir/test/Conversion/GPUToROCDL/gpu-to-rocdl.mlir
+++ b/mlir/test/Conversion/GPUToROCDL/gpu-to-rocdl.mlir
@@ -3,7 +3,7 @@
// RUN: mlir-opt %s -convert-gpu-to-rocdl='chipset=gfx950 index-bitwidth=32' -split-input-file | FileCheck --check-prefix=CHECK32 %s
// CHECK-LABEL: @test_module
-// CHECK-SAME: llvm.data_layout = "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-p7:160:256:256:32-p8:128:128:128:48-p9:192:256:256:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1-ni:7:8:9"
+// CHECK-SAME: llvm.data_layout = "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-p7:160:256:256:32-p8:128:128:128:48-p9:192:256:256:32-i64:64-i128:128-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1-ni:7:8:9"
gpu.module @test_module {
// CHECK-LABEL: func @gpu_index_ops()
``````````
</details>
https://github.com/llvm/llvm-project/pull/213523
More information about the cfe-commits
mailing list