https://github.com/farzonl created https://github.com/llvm/llvm-project/pull/214575
fixes https://github.com/llvm/llvm-project/issues/136941 This flag, mirroring DXC, opts into assuming row-major buffer bytes instead, for compatibility with legacy shaders. Outside of the driver change the raw buffers carry no layout metadata, so matrix values loaded/stored via ByteAddressBuffer::Load<T>/Store<T> are ambiguous (native layout is column-major). - New __builtin_hlsl_resource_load_typed/store_typed builtins for the templated ByteAddressBuffer Load<T>/Store<T> overloads - CodeGen transposes matrix values via llvm.matrix.transpose when the flag is set and the target is SPIR-V - Plumb flag from driver -> cc1 -> new LangOpts bit >From 4897f6b26a55bb10aa6c7f031fe6e9a53f718ef3 Mon Sep 17 00:00:00 2001 From: Farzon Lotfi <[email protected]> Date: Tue, 4 Aug 2026 15:37:46 -0400 Subject: [PATCH] [HLSL][Driver][SPIRV] Add -fspv-use-legacy-buffer-matrix-order flag fixes https://github.com/llvm/llvm-project/issues/136941 This flag, mirroring DXC, opts into assuming row-major buffer bytes instead, for compatibility with legacy shaders. Outside of the driver change the raw buffers carry no layout metadata, so matrix values loaded/stored via ByteAddressBuffer::Load<T>/Store<T> are ambiguous (native layout is column-major). - New __builtin_hlsl_resource_load_typed/store_typed builtins for the templated ByteAddressBuffer Load<T>/Store<T> overloads - CodeGen transposes matrix values via llvm.matrix.transpose when the flag is set and the target is SPIR-V - Plumb flag from driver -> cc1 -> new LangOpts bit --- clang/include/clang/Basic/Builtins.td | 12 ++++ clang/include/clang/Basic/LangOptions.def | 1 + clang/include/clang/Options/Options.td | 9 +++ clang/lib/CodeGen/CGHLSLBuiltins.cpp | 69 +++++++++++++++++++ clang/lib/Driver/ToolChains/Clang.cpp | 1 + clang/lib/Driver/ToolChains/HLSL.cpp | 9 +++ clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.cpp | 55 ++++++++++++++- clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.h | 4 ++ clang/lib/Sema/SemaHLSL.cpp | 37 ++++++++++ ...yteAddressBuffers-matrix-legacy-order.hlsl | 37 ++++++++++ ...c_fspv_use_legacy_buffer_matrix_order.hlsl | 17 +++++ 11 files changed, 249 insertions(+), 2 deletions(-) create mode 100644 clang/test/CodeGenHLSL/resources/ByteAddressBuffers-matrix-legacy-order.hlsl create mode 100644 clang/test/Driver/dxc_fspv_use_legacy_buffer_matrix_order.hlsl diff --git a/clang/include/clang/Basic/Builtins.td b/clang/include/clang/Basic/Builtins.td index 06d559a5c3ce2..74dde89e1084f 100644 --- a/clang/include/clang/Basic/Builtins.td +++ b/clang/include/clang/Basic/Builtins.td @@ -5317,6 +5317,18 @@ def HLSLResourceGetPointerTyped : Builtin { let Prototype = "void(...)"; } +def HLSLResourceLoadTyped : LangBuiltin<"HLSL_LANG"> { + let Spellings = ["__builtin_hlsl_resource_load_typed"]; + let Attributes = [NoThrow, CustomTypeChecking]; + let Prototype = "void(...)"; +} + +def HLSLResourceStoreTyped : LangBuiltin<"HLSL_LANG"> { + let Spellings = ["__builtin_hlsl_resource_store_typed"]; + let Attributes = [NoThrow, CustomTypeChecking]; + let Prototype = "void(...)"; +} + def HLSLResourceLoadWithStatus : LangBuiltin<"HLSL_LANG"> { let Spellings = ["__builtin_hlsl_resource_load_with_status"]; let Attributes = [NoThrow]; diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def index 3d63b9677e4df..3a2a12202cf09 100644 --- a/clang/include/clang/Basic/LangOptions.def +++ b/clang/include/clang/Basic/LangOptions.def @@ -248,6 +248,7 @@ ENUM_LANGOPT(HLSLVersion, HLSLLangStd, 16, HLSL_Unset, NotCompatible, "HLSL Vers LANGOPT(HLSLStrictAvailability, 1, 0, NotCompatible, "Strict availability diagnostic mode for HLSL built-in functions.") LANGOPT(HLSLSpvUseUnknownImageFormat, 1, 0, NotCompatible, "For storage images and texel buffers, sets the default format to 'Unknown' when not specified via the `vk::image_format` attribute. If this option is not used, the format is inferred from the resource's data type.") +LANGOPT(HLSLSpvUseLegacyBufferMatrixOrder, 1, 0, NotCompatible, "Assume the legacy matrix order (row major) when accessing raw buffers (e.g. ByteAddressBuffer). Defaults to column major.") LANGOPT(HLSLSpvEnableMaximalReconvergence, 1, 0, NotCompatible, "Enables the MaximallyReconvergesKHR execution mode for this module. This ensures that control flow reconverges at well-defined merge points as defined by the Vulkan spec.") LANGOPT(HLSLSpvPreserveInterface, 1, 0, NotCompatible, "Preserve entry-point interface variables from dead-code elimination.") LANGOPT(EmitLogicalPointer, 1, 0, NotCompatible, "Allow emitting structured GEP/alloca intrinsics instead of normal GEP/alloca instructions.") diff --git a/clang/include/clang/Options/Options.td b/clang/include/clang/Options/Options.td index 1478624265f79..848ec74a11513 100644 --- a/clang/include/clang/Options/Options.td +++ b/clang/include/clang/Options/Options.td @@ -9973,6 +9973,15 @@ def fhlsl_spv_use_unknown_image_format "from the resource's data type.">, MarshallingInfoFlag<LangOpts<"HLSLSpvUseUnknownImageFormat">>; +def fhlsl_spv_use_legacy_buffer_matrix_order + : Flag<["-"], "fspv-use-legacy-buffer-matrix-order">, + Group<dxc_Group>, + Visibility<[CC1Option, DXCOption]>, + HelpText<"Assume the legacy matrix order (row major) when accessing " + "raw buffers (e.g. ByteAddressBuffer). Defaults to column " + "major.">, + MarshallingInfoFlag<LangOpts<"HLSLSpvUseLegacyBufferMatrixOrder">>; + def fhlsl_spv_enable_maximal_reconvergence : Flag<["-"], "fspv-enable-maximal-reconvergence">, Group<dxc_Group>, diff --git a/clang/lib/CodeGen/CGHLSLBuiltins.cpp b/clang/lib/CodeGen/CGHLSLBuiltins.cpp index 0796d121a9b43..2531e102e2b37 100644 --- a/clang/lib/CodeGen/CGHLSLBuiltins.cpp +++ b/clang/lib/CodeGen/CGHLSLBuiltins.cpp @@ -647,6 +647,75 @@ Value *CodeGenFunction::EmitHLSLBuiltinExpr(unsigned BuiltinID, CI->setCallingConv(IntrFn->getCallingConv()); return CI; } + case Builtin::BI__builtin_hlsl_resource_load_typed: { + Value *HandleOp = EmitScalarExpr(E->getArg(0)); + Value *IndexOp = EmitScalarExpr(E->getArg(1)); + + QualType ElemQTy = E->getType(); + QualType PtrQTy = getContext().getPointerType( + getContext().getAddrSpaceQualType(ElemQTy, LangAS::hlsl_device)); + llvm::Type *PtrTy = ConvertType(PtrQTy); + llvm::Type *ElemTy = ConvertType(ElemQTy); + + llvm::Function *IntrFn = llvm::Intrinsic::getOrInsertDeclaration( + &CGM.getModule(), + CGM.getHLSLRuntime().getCreateResourceGetPointerIntrinsic(), + {PtrTy, HandleOp->getType(), IndexOp->getType()}); + llvm::CallInst *Ptr = EmitRuntimeCall(IntrFn, {HandleOp, IndexOp}); + Ptr->setCallingConv(IntrFn->getCallingConv()); + + Address Addr(Ptr, ElemTy, getContext().getTypeAlignInChars(ElemQTy)); + Value *Loaded = Builder.CreateLoad(Addr); + + // Legacy row-major buffer order loads the transposed shape; transpose back. + if (const auto *MatTy = ElemQTy->getAs<ConstantMatrixType>()) { + const HLSLAttributedResourceType *RT = getRequiredHandleType(E, 0); + if (RT->isRaw() && CGM.getTriple().isSPIRV() && + getLangOpts().HLSLSpvUseLegacyBufferMatrixOrder) { + llvm::MatrixBuilder MB(Builder); + Loaded = MB.CreateMatrixTranspose(Loaded, MatTy->getNumColumns(), + MatTy->getNumRows()); + } + } + return Loaded; + } + case Builtin::BI__builtin_hlsl_resource_store_typed: { + // Value is evaluated before Handle/Index to match the IR shape of the + // other (non-templated) Store overloads. + Value *ValueOp = EmitScalarExpr(E->getArg(2)); + Value *HandleOp = EmitScalarExpr(E->getArg(0)); + Value *IndexOp = EmitScalarExpr(E->getArg(1)); + + QualType ElemQTy = E->getArg(2)->getType(); + + // See the load_typed case above. + if (const auto *MatTy = ElemQTy->getAs<ConstantMatrixType>()) { + const HLSLAttributedResourceType *RT = getRequiredHandleType(E, 0); + if (RT->isRaw() && CGM.getTriple().isSPIRV() && + getLangOpts().HLSLSpvUseLegacyBufferMatrixOrder) { + llvm::MatrixBuilder MB(Builder); + ValueOp = MB.CreateMatrixTranspose(ValueOp, MatTy->getNumRows(), + MatTy->getNumColumns()); + } + } + + QualType PtrQTy = getContext().getPointerType( + getContext().getAddrSpaceQualType(ElemQTy, LangAS::hlsl_device)); + llvm::Type *PtrTy = ConvertType(PtrQTy); + + llvm::Function *IntrFn = llvm::Intrinsic::getOrInsertDeclaration( + &CGM.getModule(), + CGM.getHLSLRuntime().getCreateResourceGetPointerIntrinsic(), + {PtrTy, HandleOp->getType(), IndexOp->getType()}); + llvm::CallInst *Ptr = EmitRuntimeCall(IntrFn, {HandleOp, IndexOp}); + Ptr->setCallingConv(IntrFn->getCallingConv()); + + Address Addr(Ptr, ValueOp->getType(), + getContext().getTypeAlignInChars(ElemQTy)); + // Returning the (void-typed) store instruction, rather than nullptr, + // signals to the caller that this builtin was handled. + return Builder.CreateStore(ValueOp, Addr); + } case Builtin::BI__builtin_hlsl_resource_sample: { Value *HandleOp = EmitScalarExpr(E->getArg(0)); Value *SamplerOp = EmitScalarExpr(E->getArg(1)); diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 94f9a26aac39f..8923a4c4f868e 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -3979,6 +3979,7 @@ static void RenderHLSLOptions(const Driver &D, const ArgList &Args, options::OPT_fdx_rootsignature_define, options::OPT_fdx_rootsignature_version, options::OPT_fhlsl_spv_use_unknown_image_format, + options::OPT_fhlsl_spv_use_legacy_buffer_matrix_order, options::OPT_fhlsl_spv_enable_maximal_reconvergence, options::OPT_fhlsl_spv_preserve_interface}; if (!types::isHLSL(InputType)) diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp index 558408cc969f0..9e7005b8798f2 100644 --- a/clang/lib/Driver/ToolChains/HLSL.cpp +++ b/clang/lib/Driver/ToolChains/HLSL.cpp @@ -533,6 +533,15 @@ HLSLToolChain::TranslateArgs(const DerivedArgList &Args, BoundArch BA, continue; } + if (A->getOption().getID() == + options::OPT_fhlsl_spv_use_legacy_buffer_matrix_order && + getArch() != llvm::Triple::spirv) { + getDriver().Diag(diag::err_drv_argument_only_allowed_with) + << A->getAsString(Args) << "-spirv"; + A->claim(); + continue; + } + if (A->getOption().getID() == options::OPT_enable_16bit_types) { // Translate -enable-16bit-types into -fnative-half-type and // -fnative-int16-type diff --git a/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.cpp b/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.cpp index c788a153f730e..33aa29dbf8d13 100644 --- a/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.cpp +++ b/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.cpp @@ -1609,7 +1609,16 @@ BuiltinTypeDeclBuilder::addByteAddressBufferLoadMethods() { AddLoads("Load2", AST.getExtVectorType(AST.UnsignedIntTy, 2)); AddLoads("Load3", AST.getExtVectorType(AST.UnsignedIntTy, 3)); AddLoads("Load4", AST.getExtVectorType(AST.UnsignedIntTy, 4)); - AddLoads("Load", AST.DependentTy); // Templated version + + // Templated Load<T>() needs buffer-order-aware handling for matrix T; see + // __builtin_hlsl_resource_load_typed in CGHLSLBuiltins.cpp. + { + IdentifierInfo &II = AST.Idents.get("Load", tok::TokenKind::identifier); + DeclarationName Load(&II); + addRawBufferGenericLoadFunction(Load); + addLoadWithStatusFunction(Load, AST.DependentTy); + } + return *this; } @@ -1630,7 +1639,13 @@ BuiltinTypeDeclBuilder::addByteAddressBufferStoreMethods() { AddStore("Store2", AST.getExtVectorType(AST.UnsignedIntTy, 2)); AddStore("Store3", AST.getExtVectorType(AST.UnsignedIntTy, 3)); AddStore("Store4", AST.getExtVectorType(AST.UnsignedIntTy, 4)); - AddStore("Store", AST.DependentTy); // Templated version + + // Templated Store<T>(); see addByteAddressBufferLoadMethods() above. + { + IdentifierInfo &II = AST.Idents.get("Store", tok::TokenKind::identifier); + DeclarationName Store(&II); + addRawBufferGenericStoreFunction(Store); + } return *this; } @@ -2321,6 +2336,42 @@ BuiltinTypeDeclBuilder::addLoadWithStatusFunction(DeclarationName &Name, return MMB.finalize(); } +BuiltinTypeDeclBuilder & +BuiltinTypeDeclBuilder::addRawBufferGenericLoadFunction(DeclarationName &Name) { + assert(!Record->isCompleteDefinition() && "record is already complete"); + ASTContext &AST = SemaRef.getASTContext(); + using PH = BuiltinTypeMethodBuilder::PlaceHolder; + + // The empty QualType is a placeholder. The actual return type is set below + // once the template parameter is created. This method is always const; + // it does not rebind the resource handle. + BuiltinTypeMethodBuilder MMB(*this, Name, QualType(), /*IsConst=*/true); + QualType ElemTy = MMB.addTemplateTypeParam("element_type"); + MMB.ReturnTy = ElemTy; + + return MMB.addParam("Index", AST.UnsignedIntTy) + .callBuiltin("__builtin_hlsl_resource_load_typed", ElemTy, PH::Handle, + PH::_0, ElemTy) + .finalize(); +} + +BuiltinTypeDeclBuilder & +BuiltinTypeDeclBuilder::addRawBufferGenericStoreFunction( + DeclarationName &Name) { + assert(!Record->isCompleteDefinition() && "record is already complete"); + ASTContext &AST = SemaRef.getASTContext(); + using PH = BuiltinTypeMethodBuilder::PlaceHolder; + + BuiltinTypeMethodBuilder MMB(*this, Name, AST.VoidTy, /*IsConst=*/false); + QualType ElemTy = MMB.addTemplateTypeParam("element_type"); + + return MMB.addParam("Index", AST.UnsignedIntTy) + .addParam("Value", ElemTy) + .callBuiltin("__builtin_hlsl_resource_store_typed", AST.VoidTy, + PH::Handle, PH::_0, PH::_1) + .finalize(); +} + BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addHandleAccessFunction( DeclarationName &Name, bool IsConstReturn, bool IsRef, QualType IndexTy, QualType ElemTy) { diff --git a/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.h b/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.h index e809ef264198c..094e023b7f12c 100644 --- a/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.h +++ b/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.h @@ -136,6 +136,10 @@ class BuiltinTypeDeclBuilder { BuiltinTypeDeclBuilder &addStoreFunction(DeclarationName &Name, bool IsConst, QualType ValueType); BuiltinTypeDeclBuilder & + addRawBufferGenericLoadFunction(DeclarationName &Name); + BuiltinTypeDeclBuilder & + addRawBufferGenericStoreFunction(DeclarationName &Name); + BuiltinTypeDeclBuilder & addByteAddressBufferInterlockedMethod(StringRef MethodName, QualType ValueTy, StringRef BuiltinName); BuiltinTypeDeclBuilder &addAppendMethod(); diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index 088d1a50359a5..d793d5fb0e440 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -4129,6 +4129,43 @@ bool SemaHLSL::CheckBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall) { break; } + case Builtin::BI__builtin_hlsl_resource_load_typed: { + if (SemaRef.checkArgCount(TheCall, 3) || + CheckResourceHandle(&SemaRef, TheCall, 0) || + CheckIndexType(&SemaRef, TheCall, 1)) + return true; + + QualType ReturnType = TheCall->getArg(2)->getType(); + assert(ReturnType->isPointerType() && + "expected pointer type for the third argument"); + ReturnType = ReturnType->getPointeeType(); + + // Reject array types + if (ReturnType->isArrayType()) + return SemaRef.Diag( + cast<FunctionDecl>(SemaRef.CurContext)->getPointOfInstantiation(), + diag::err_invalid_use_of_array_type); + + TheCall->setType(ReturnType); + + break; + } + case Builtin::BI__builtin_hlsl_resource_store_typed: { + if (SemaRef.checkArgCount(TheCall, 3) || + CheckResourceHandle(&SemaRef, TheCall, 0) || + CheckIndexType(&SemaRef, TheCall, 1)) + return true; + + // Reject array types + if (TheCall->getArg(2)->getType()->isArrayType()) + return SemaRef.Diag( + cast<FunctionDecl>(SemaRef.CurContext)->getPointOfInstantiation(), + diag::err_invalid_use_of_array_type); + + TheCall->setType(SemaRef.getASTContext().VoidTy); + + break; + } case Builtin::BI__builtin_hlsl_resource_load_with_status: { if (SemaRef.checkArgCount(TheCall, 3) || CheckResourceHandle(&SemaRef, TheCall, 0) || diff --git a/clang/test/CodeGenHLSL/resources/ByteAddressBuffers-matrix-legacy-order.hlsl b/clang/test/CodeGenHLSL/resources/ByteAddressBuffers-matrix-legacy-order.hlsl new file mode 100644 index 0000000000000..e1ee18a697373 --- /dev/null +++ b/clang/test/CodeGenHLSL/resources/ByteAddressBuffers-matrix-legacy-order.hlsl @@ -0,0 +1,37 @@ +// RUN: %clang_cc1 -triple spirv-unknown-vulkan-library -finclude-default-header \ +// RUN: -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s --check-prefixes=CHECK,DEFAULT +// RUN: %clang_cc1 -triple spirv-unknown-vulkan-library -finclude-default-header \ +// RUN: -fspv-use-legacy-buffer-matrix-order -emit-llvm -disable-llvm-passes -o - %s \ +// RUN: | FileCheck %s --check-prefixes=CHECK,LEGACY + +// Raw buffers carry no layout information. By default, a matrix read from or +// written to a raw buffer is assumed to be stored in column-major order, +// matching the native in-register representation, so no reordering is +// needed. -fspv-use-legacy-buffer-matrix-order assumes the raw bytes are +// stored in row-major order instead, which requires transposing the loaded +// (or, before storing, the to-be-stored) value. + +ByteAddressBuffer Buf : register(t0); +RWByteAddressBuffer RWBuf : register(u0); + +export float2x3 TestLoad() { + return Buf.Load<float2x3>(0); +} + +// CHECK-LABEL: define {{.*}} <6 x float> @{{.*}}ByteAddressBuffer4LoadIu11matrix_typeILj2ELj3EfEEET_j +// CHECK: [[LOADED:%.*]] = load <6 x float>, ptr addrspace(11) %{{.*}} +// DEFAULT-NOT: call {{.*}} @llvm.matrix.transpose +// DEFAULT: ret <6 x float> [[LOADED]] +// LEGACY: [[TRANSPOSED:%.*]] = call {{.*}} <6 x float> @llvm.matrix.transpose.v6f32(<6 x float> [[LOADED]], i32 3, i32 2) +// LEGACY: ret <6 x float> [[TRANSPOSED]] + +export void TestStore(float2x3 M) { + RWBuf.Store<float2x3>(0, M); +} + +// CHECK-LABEL: define {{.*}} void @{{.*}}RWByteAddressBuffer5StoreIu11matrix_typeILj2ELj3EfEEEvjT_ +// CHECK: [[VALUE:%.*]] = load <6 x float>, ptr %Value.addr +// DEFAULT-NOT: call {{.*}} @llvm.matrix.transpose +// DEFAULT: store <6 x float> [[VALUE]], ptr addrspace(11) %{{.*}} +// LEGACY: [[TRANSPOSED:%.*]] = call {{.*}} <6 x float> @llvm.matrix.transpose.v6f32(<6 x float> [[VALUE]], i32 2, i32 3) +// LEGACY: store <6 x float> [[TRANSPOSED]], ptr addrspace(11) %{{.*}} diff --git a/clang/test/Driver/dxc_fspv_use_legacy_buffer_matrix_order.hlsl b/clang/test/Driver/dxc_fspv_use_legacy_buffer_matrix_order.hlsl new file mode 100644 index 0000000000000..f63985c7745a6 --- /dev/null +++ b/clang/test/Driver/dxc_fspv_use_legacy_buffer_matrix_order.hlsl @@ -0,0 +1,17 @@ +// Verify that -fspv-use-legacy-buffer-matrix-order is accepted by the driver +// and forwarded to cc1 as -fspv-use-legacy-buffer-matrix-order. +// RUN: %clang_dxc -spirv -Tlib_6_7 -fspv-use-legacy-buffer-matrix-order -### %s 2>&1 \ +// RUN: | FileCheck %s --check-prefix=CHECK-LEGACY +// CHECK-LEGACY: "-fspv-use-legacy-buffer-matrix-order" + +// Without the flag, -fspv-use-legacy-buffer-matrix-order must not appear in +// cc1 args. +// RUN: %clang_dxc -spirv -Tlib_6_7 -### %s 2>&1 \ +// RUN: | FileCheck %s --check-prefix=CHECK-NO-LEGACY +// CHECK-NO-LEGACY-NOT: "-fspv-use-legacy-buffer-matrix-order" + +// The flag requires -spirv. +// RUN: not %clang_dxc -Tlib_6_7 -fspv-use-legacy-buffer-matrix-order -### %s 2>&1 \ +// RUN: | FileCheck %s --check-prefix=CHECK-NO-SPIRV +// CHECK-NO-SPIRV: error: invalid argument '-fspv-use-legacy-buffer-matrix-order' only allowed with '-spirv' + _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
