[clang] [llvm] [mlir] [AMDGPU] fix alignment of i128 (PR #213523)
Simeon David Schaub via cfe-commits
cfe-commits at lists.llvm.org
Sun Aug 2 03:16:39 PDT 2026
https://github.com/simeonschaub created https://github.com/llvm/llvm-project/pull/213523
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)
>From 9d08f060ca5c8b42b08b1990fc839a400da354c0 Mon Sep 17 00:00:00 2001
From: Simeon David Schaub <simeon at schaub.rocks>
Date: Sun, 2 Aug 2026 10:01:56 +0000
Subject: [PATCH] [AMDGPU] Give i128 an ABI alignment of 16 in the data layout
The AMDGPU data layout has no entry for i128, so `DataLayout` falls back
to the next-lower integer entry (`i64:64`) and gives i128 an ABI
alignment of 8. Clang, however, aligns `__int128` to 16 bytes on AMDGPU:
`AMDGPUTargetInfo` never overrides `Int128Align`, so it keeps the
128-bit default from `TargetInfo`. NVPTX already spells `i128:128` out
in `computeNVPTXDataLayout` for the same reason.
Any frontend that computes aggregate offsets itself and then emits an
LLVM struct type without explicit padding therefore ends up with a
layout that LLVM disagrees with. For kernel arguments that is an ABI
break rather than a missed optimization: `getExplicitKernArgSize()`
sizes the kernarg slot from the data layout, so the tail of an argument
containing an i128 is never copied into the kernarg segment, and loads
of it read past the end of the segment. A kernel taking
`struct { long a; __int128 b; }` by reference gets a 24-byte slot for a
32-byte object, and the load of `b` runs 8 bytes off the end.
Add the missing `i128:128` entry, and auto-upgrade existing AMDGCN data
layout strings that lack one. r600 is left alone: its pointers are
32-bit, so `TargetInfo::hasInt128Type()` is false there and Clang never
forms an `__int128`.
Assisted-by: Claude Code (claude-opus-5)
---
clang/test/CodeGen/target-data.c | 4 +-
clang/test/CodeGenOpenCL/amdgpu-env-amdgcn.cl | 2 +-
llvm/lib/IR/AutoUpgrade.cpp | 6 ++
llvm/lib/TargetParser/TargetDataLayout.cpp | 10 ++-
.../CodeGen/AMDGPU/kernarg-i128-alignment.ll | 82 +++++++++++++++++++
.../Bitcode/DataLayoutUpgradeTest.cpp | 34 +++++---
.../Conversion/GPUToROCDL/gpu-to-rocdl.mlir | 2 +-
7 files changed, 123 insertions(+), 17 deletions(-)
create mode 100644 llvm/test/CodeGen/AMDGPU/kernarg-i128-alignment.ll
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()
More information about the cfe-commits
mailing list