From be1b929abed4d5c3f00e737bf849bfe791d96905 Mon Sep 17 00:00:00 2001 From: Paulius Velesko Date: Fri, 31 Jul 2026 17:49:47 +0300 Subject: [PATCH 1/2] Tests: LowerByValAttribute must honour the byval alignment attribute LowerByValAttribute replaces a byval pointer argument with a local copy. The copy is created with builder.CreateAlloca(ElTy) and the memcpy uses DL.getABITypeAlign(ElTy); neither consults the alignment stated in the `byval(%T) align N` attribute. For `{[8 x double]}` the ABI alignment is 8, so a parameter declared `byval align 64` loses the guarantee its callee is entitled to rely on. byval-explicit-alignment.ll checks both directions: the copy of the over-aligned argument must come out `align 64`, and the copy of a parameter with no explicit alignment must keep exactly the alignment it has today, `align 8` - the preferred alignment CreateAlloca() gives an aggregate under the default data layout, not its ABI alignment of 4. The second case is there so that a fix cannot raise one alignment by lowering the other. Expected to fail until the attribute's alignment is propagated. Related: intel/intel-graphics-compiler#392. This is a second, latent instance of the same class of bug; the reproducer in that issue takes a different path (its parameters are readonly, which this pass short-circuits). Signed-off-by: Paulius Velesko --- .../byval-explicit-alignment.ll | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 IGC/Compiler/tests/LowerByValAttribute/byval-explicit-alignment.ll diff --git a/IGC/Compiler/tests/LowerByValAttribute/byval-explicit-alignment.ll b/IGC/Compiler/tests/LowerByValAttribute/byval-explicit-alignment.ll new file mode 100644 index 000000000000..4692d6fcd6b1 --- /dev/null +++ b/IGC/Compiler/tests/LowerByValAttribute/byval-explicit-alignment.ll @@ -0,0 +1,58 @@ +;=========================== begin_copyright_notice ============================ +; +; Copyright (C) 2026 Intel Corporation +; +; SPDX-License-Identifier: MIT +; +;============================ end_copyright_notice ============================= +; +; REQUIRES: llvm-16-plus +; RUN: igc_opt --opaque-pointers %s -S -o - --igc-lower-byval-attribute | FileCheck %s + +; ------------------------------------------------ +; LowerByValAttribute +; ------------------------------------------------ +; +; The hidden copy that this pass materialises for a `byval` argument must honour +; the alignment stated by the attribute, not just the ABI alignment of the +; pointee type. `%struct.double8` is a struct of doubles, so its ABI alignment is +; only 8, but the argument is declared `align 64` and the callee (and every pass +; downstream) is entitled to rely on that. Creating the alloca with the default +; alignment leaves the copy 8-byte aligned and the callee then reads the wrong +; bytes. + +%struct.double8 = type { double, double, double, double, double, double, double, double } +%struct.plain = type { i32, [10 x i32], i32 } + +define spir_kernel void @kernel(ptr byval(%struct.double8) align 64 %s, ptr byval(%struct.plain) %p) { +; CHECK-LABEL: @kernel( + +; COM: alignment 64 comes from the byval attribute, not from the struct layout +; CHECK: [[ALLOCA0:%.*]] = alloca %struct.double8, align 64 +; CHECK: call void @llvm.memcpy.p0.p0.i64(ptr align 64 [[ALLOCA0]], ptr align 64 %s, i64 64, i1 false) +; CHECK: call void @f0(ptr byval(%struct.double8) align 64 [[ALLOCA0]]) + call void @f0(ptr byval(%struct.double8) align 64 %s) + +; COM: without an explicit alignment on the attribute nothing changes: the alloca +; COM: keeps the preferred alignment CreateAlloca() gives it (8 for an aggregate +; COM: under the default data layout, even though the ABI alignment is only 4), +; COM: and the source is still only assumed to be ABI aligned. +; CHECK: [[ALLOCA1:%.*]] = alloca %struct.plain, align 8 +; CHECK: call void @llvm.memcpy.p0.p0.i64(ptr align 8 [[ALLOCA1]], ptr align 4 %p, i64 48, i1 false) +; CHECK: call void @f1(ptr byval(%struct.plain) [[ALLOCA1]]) + call void @f1(ptr byval(%struct.plain) %p) + + ret void +} + +define spir_func void @f0(ptr byval(%struct.double8) align 64 %src) #0 { + store double 2.000000e+00, ptr %src, align 64 + ret void +} + +define spir_func void @f1(ptr byval(%struct.plain) %src) #0 { + store i32 222, ptr %src, align 4 + ret void +} + +attributes #0 = { noinline } From 9bb288a199558bc117edc995b25554983b953973 Mon Sep 17 00:00:00 2001 From: Paulius Velesko Date: Fri, 31 Jul 2026 17:49:47 +0300 Subject: [PATCH 2/2] LowerByValAttribute: propagate the byval alignment to the local copy The hidden copy that replaces a byval pointer argument was created with builder.CreateAlloca(ElTy) and memcpy'd with DL.getABITypeAlign(ElTy), neither of which looks at the alignment stated in the attribute itself. For `byval({[8 x double]}) align 64` that gives a copy aligned to 8. The callee is entitled to rely on the alignment the attribute promises: downstream codegen folds constant offsets into a bitwise OR on the low half of the pointer, which is only valid when the low bits are zero. An under-aligned copy therefore breaks that entitlement. Raise the alloca to the attribute's alignment via std::max against the alignment it already has: CreateAlloca() gives the *preferred* alignment of the element type, which for an aggregate is 8 under the default data layout even when its ABI alignment is only 4, so the alignment must only ever be raised, never overwritten with max(ABI, attribute), which would silently demote every type whose preferred alignment exceeds its ABI alignment. The memcpy's destination alignment follows the alloca; its source alignment stays at what is actually known about the incoming pointer, which is the attribute's alignment when there is one and the ABI alignment otherwise. Where the attribute states no alignment, nothing changes. Fixes the test added in the preceding commit. Signed-off-by: Paulius Velesko --- .../OpenCLPasses/PrivateMemory/LowerByValAttribute.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/IGC/Compiler/Optimizer/OpenCLPasses/PrivateMemory/LowerByValAttribute.cpp b/IGC/Compiler/Optimizer/OpenCLPasses/PrivateMemory/LowerByValAttribute.cpp index a5bd8d48304e..58bf9934c494 100644 --- a/IGC/Compiler/Optimizer/OpenCLPasses/PrivateMemory/LowerByValAttribute.cpp +++ b/IGC/Compiler/Optimizer/OpenCLPasses/PrivateMemory/LowerByValAttribute.cpp @@ -16,6 +16,7 @@ SPDX-License-Identifier: MIT #include "common/LLVMWarningsPop.hpp" #include #include +#include using namespace llvm; using namespace IGC; @@ -69,9 +70,11 @@ void LowerByValAttribute::visitCallInst(CallInst &CI) { if (CI.paramHasAttr(i, llvm::Attribute::ByVal) && !CI.paramHasAttr(i, llvm::Attribute::ReadOnly)) { Type *ElTy = CI.getParamByValType(i); + Align SrcAlign = std::max(DL.getABITypeAlign(ElTy), CI.getParamAlign(i).valueOrOne()); IGCLLVM::IRBuilder<> builder(&CI); - Value *AI = builder.CreateAlloca(ElTy); - builder.CreateMemCpy(AI, OpI, DL.getTypeAllocSize(ElTy), DL.getABITypeAlign(ElTy).value()); + AllocaInst *AI = builder.CreateAlloca(ElTy); + AI->setAlignment(std::max(AI->getAlign(), SrcAlign)); + builder.CreateMemCpy(AI, AI->getAlign(), OpI, SrcAlign, DL.getTypeAllocSize(ElTy)); auto AC = builder.CreateAddrSpaceCast(AI, OpITy); CI.replaceUsesOfWith(OpI, AC); m_changed = true;