From ed7272a627b190c8afa9090311e586a97df895d3 Mon Sep 17 00:00:00 2001 From: Alexey Bader Date: Mon, 24 Nov 2025 18:59:07 -0800 Subject: [PATCH] [SYCLLowerIR] Fix metadata format for Intel cache controls This patch aligns metadata with the format defined in the commit message for https://github.com/KhronosGroup/SPIRV-LLVM-Translator/commit/c8bfc33ef8f9f273546a49d87a6612de0791e3d4. Unfortunately, the format is still not documented in https://github.com/KhronosGroup/SPIRV-LLVM-Translator/blob/main/docs/SPIRVRepresentationInLLVM.rst. This patch fixes the crash in the translator tool, which tries to handle wrong metadata format. I manually checked that the translator successfully converts properties_cache_control.cpp test to SPIR-V format with CacheControl decorations. --- .../SYCLLowerIR/CompileTimePropertiesPass.cpp | 64 +++++++------ .../properties/properties_cache_control.cpp | 89 ++++++++++--------- 2 files changed, 82 insertions(+), 71 deletions(-) diff --git a/llvm/lib/SYCLLowerIR/CompileTimePropertiesPass.cpp b/llvm/lib/SYCLLowerIR/CompileTimePropertiesPass.cpp index ca79ecc028915..c049ad34434f2 100644 --- a/llvm/lib/SYCLLowerIR/CompileTimePropertiesPass.cpp +++ b/llvm/lib/SYCLLowerIR/CompileTimePropertiesPass.cpp @@ -33,6 +33,8 @@ constexpr StringRef SyclRegisterAllocModeAttr = "sycl-register-alloc-mode"; constexpr StringRef SyclGrfSizeAttr = "sycl-grf-size"; constexpr StringRef SpirvDecorMdKind = "spirv.Decorations"; +constexpr StringRef SpirvDecorCacheControlMdKind = + "spirv.DecorationCacheControlINTEL"; constexpr StringRef SpirvParamDecorMdKind = "spirv.ParameterDecorations"; // The corresponding SPIR-V OpCode for the host_access property is documented // in the SPV_INTEL_global_variable_decorations design document: @@ -128,21 +130,7 @@ MDNode *buildSpirvDecorMetadata(LLVMContext &Ctx, uint32_t OpCode, return MDNode::get(Ctx, MD); } -/// Builds a metadata node for a SPIR-V decoration for cache controls -/// where decoration code and value are both uint32_t integers. -/// The value encodes a cache level and a cache control type. -/// -/// @param Ctx [in] the LLVM Context. -/// @param Name [in] the SPIR-V property string name. -/// @param OpCode [in] the SPIR-V opcode. -/// @param CacheMode [in] whether read or write. -/// @param CacheLevel [in] the cache level. -/// -/// @returns a pointer to the metadata node created for the required decoration -/// and its values. -MDNode *buildSpirvDecorCacheProp(LLVMContext &Ctx, StringRef Name, - uint32_t OpCode, uint32_t CacheMode, - uint32_t CacheLevel) { +static uint32_t getCacheProperty(StringRef Name, uint32_t CacheMode) { // SPIR-V encodings of read control enum CacheControlReadType { read_uncached = 0, @@ -177,12 +165,25 @@ MDNode *buildSpirvDecorCacheProp(LLVMContext &Ctx, StringRef Name, write_uncached, write_through, write_back}; // Map SYCL encoding to SPIR-V - uint32_t CacheProp; if (Name.starts_with("sycl-cache-read")) - CacheProp = SPIRVReadControl[CacheMode]; - else - CacheProp = SPIRVWriteControl[CacheMode]; + return SPIRVReadControl[CacheMode]; + + return SPIRVWriteControl[CacheMode]; +} +/// Builds a metadata node for a SPIR-V decoration for cache controls. +/// +/// @param Ctx [in] the LLVM Context. +/// @param OpCode [in] the SPIR-V opcode. +/// @param CacheLevel [in] the cache level. +/// @param CacheProp [in] the cache property. +/// @param OperandNum [in] the operand number to decorate. +/// +/// @returns a pointer to the metadata node created for the required decoration +/// and its values. +MDNode *buildSpirvDecorCacheProp(LLVMContext &Ctx, uint32_t OpCode, + uint32_t CacheLevel, uint32_t CacheProp, + uint32_t OperandNum) { auto *Ty = Type::getInt32Ty(Ctx); SmallVector MD; MD.push_back(ConstantAsMetadata::get( @@ -191,6 +192,8 @@ MDNode *buildSpirvDecorCacheProp(LLVMContext &Ctx, StringRef Name, Constant::getIntegerValue(Ty, APInt(32, CacheLevel)))); MD.push_back(ConstantAsMetadata::get( Constant::getIntegerValue(Ty, APInt(32, CacheProp)))); + MD.push_back(ConstantAsMetadata::get( + Constant::getIntegerValue(Ty, APInt(32, OperandNum)))); return MDNode::get(Ctx, MD); } @@ -831,7 +834,7 @@ bool CompileTimePropertiesPass::transformSYCLPropertiesAnnotation( // Read the annotation values and create new annotation strings. std::string NewAnnotString = ""; auto Properties = parseSYCLPropertiesString(M, IntrInst); - SmallVector MDOpsCacheProp; + SmallVector, 8> MDOpsCacheProp; bool CacheProp = false; bool FPGAProp = false; for (const auto &[PropName, PropVal] : Properties) { @@ -856,7 +859,6 @@ bool CompileTimePropertiesPass::transformSYCLPropertiesAnnotation( // !CC1 = !{i32 Load/Store, i32 Level, i32 Control} // !CC2 = !{i32 Load/Store, i32 Level, i32 Control} // ... - LLVMContext &Ctx = M.getContext(); uint32_t CacheMode = 0; while (AttrVal) { // The attribute value encodes cache control and levels. @@ -869,8 +871,8 @@ bool CompileTimePropertiesPass::transformSYCLPropertiesAnnotation( uint32_t LevelMask = AttrVal & 0xf; while (LevelMask) { if (LevelMask & 1) - MDOpsCacheProp.push_back(buildSpirvDecorCacheProp( - Ctx, *PropName, DecorCode, CacheMode, CacheLevel)); + MDOpsCacheProp.push_back({DecorCode, CacheLevel, + getCacheProperty(*PropName, CacheMode)}); ++CacheLevel; LevelMask >>= 1; } @@ -941,7 +943,7 @@ bool CompileTimePropertiesPass::transformSYCLPropertiesAnnotation( if (CacheProp) { LLVMContext &Ctx = M.getContext(); - unsigned MDKindID = Ctx.getMDKindID(SpirvDecorMdKind); + unsigned MDKindID = Ctx.getMDKindID(SpirvDecorCacheControlMdKind); if (!FPGAProp && llvm::isa(IntrInst->getArgOperand(0))) { // Find all load/store instructions using the pointer being annotated and // apply the cache control metadata to them. @@ -950,15 +952,15 @@ bool CompileTimePropertiesPass::transformSYCLPropertiesAnnotation( getUserListIgnoringCast(IntrInst, TargetedInstList); getUserListIgnoringCast(IntrInst, TargetedInstList); for (const auto &[Inst, MDVal] : TargetedInstList) { + assert(MDVal >= 0 && "Invalid operand number for instruction."); // Merge with existing metadata if present. SmallVector MDOps; if (MDNode *CurrentMD = Inst->getMetadata(MDKindID)) for (Metadata *Op : CurrentMD->operands()) MDOps.push_back(Op); - for (Metadata *Op : MDOpsCacheProp) - MDOps.push_back(Op); - MDOps.push_back(ConstantAsMetadata::get(Constant::getIntegerValue( - Type::getInt32Ty(Ctx), APInt(32, MDVal)))); + for (const std::array &Op : MDOpsCacheProp) + MDOps.push_back(buildSpirvDecorCacheProp(Ctx, Op[0], Op[1], Op[2], + uint32_t(MDVal))); Inst->setMetadata(MDKindID, MDTuple::get(Ctx, MDOps)); } // Replace all uses of ptr.annotations intrinsic with first operand and @@ -969,7 +971,11 @@ bool CompileTimePropertiesPass::transformSYCLPropertiesAnnotation( } else { // If there were FPGA annotations then we retain the original intrinsic // and apply the cache control properties to its result. - IntrInst->setMetadata(MDKindID, MDTuple::get(Ctx, MDOpsCacheProp)); + SmallVector MDOps; + for (const std::array &Op : MDOpsCacheProp) + MDOps.push_back( + buildSpirvDecorCacheProp(Ctx, Op[0], Op[1], Op[2], uint32_t(0))); + IntrInst->setMetadata(MDKindID, MDTuple::get(Ctx, MDOps)); } } diff --git a/sycl/test/check_device_code/extensions/properties/properties_cache_control.cpp b/sycl/test/check_device_code/extensions/properties/properties_cache_control.cpp index 6d1b21b41fe37..ed244547ada57 100644 --- a/sycl/test/check_device_code/extensions/properties/properties_cache_control.cpp +++ b/sycl/test/check_device_code/extensions/properties/properties_cache_control.cpp @@ -171,62 +171,67 @@ SYCL_EXTERNAL void annotated_ptr_func_param_test(float *p) { } // CHECK: spir_func{{.*}}annotated_ptr_func_param_test -// CHECK: store float 4.200000e+01, ptr addrspace(4) %{{.*}}, !spirv.Decorations ![[WHINT:[0-9]+]] +// CHECK: store float 4.200000e+01, ptr addrspace(4) %{{.*}}, !spirv.DecorationCacheControlINTEL ![[WHINT:[0-9]+]] // CHECK: ret void // CHECK: spir_kernel{{.*}}cache_control_read_hint_func -// CHECK: store float 5.500000e+01, ptr addrspace(1) %{{.*}}, !spirv.Decorations ![[RHINT:[0-9]+]] +// CHECK: store float 5.500000e+01, ptr addrspace(1) %{{.*}}, !spirv.DecorationCacheControlINTEL ![[RHINT:[0-9]+]] // CHECK: ret void // CHECK: spir_kernel{{.*}}cache_control_read_assertion_func -// CHECK: store i32 66, ptr addrspace(1) %{{.*}}, !spirv.Decorations ![[RASSERT:[0-9]+]] +// CHECK: store i32 66, ptr addrspace(1) %{{.*}}, !spirv.DecorationCacheControlINTEL ![[RASSERT:[0-9]+]] // CHECK: ret void // CHECK: spir_kernel{{.*}}cache_control_write_hint_func -// CHECK: store float 7.700000e+01, ptr addrspace(1) %{{.*}}, !spirv.Decorations ![[WHINT]] +// CHECK: store float 7.700000e+01, ptr addrspace(1) %{{.*}}, !spirv.DecorationCacheControlINTEL ![[WHINT]] // CHECK: ret void // CHECK: spir_kernel{{.*}}cache_control_read_write_func -// CHECK: store float 7.700000e+01, ptr addrspace(1) %{{.*}}, !spirv.Decorations ![[RWHINT:[0-9]+]] +// CHECK: store float 7.700000e+01, ptr addrspace(1) %{{.*}}, !spirv.DecorationCacheControlINTEL ![[RWHINT:[0-9]+]] // CHECK: ret void // CHECK: spir_kernel{{.*}}cache_control_load_store_func -// CHECK: store double 1.000000e+00, ptr addrspace(1) %[[PTR_A:.*]], align 8{{.*}}, !spirv.Decorations ![[STHINT_A:[0-9]+]] -// CHECK: store double 1.000000e+00, ptr addrspace(1) %[[PTR_B:.*]], align 8{{.*}}, !spirv.Decorations ![[STHINT_B:[0-9]+]] -// CHECK: load double, ptr addrspace(1) %[[PTR_A]], align 8{{.*}}, !spirv.Decorations ![[LDHINT_A:[0-9]+]] -// CHECK: load double, ptr addrspace(1) %[[PTR_B]], align 8{{.*}}, !spirv.Decorations ![[LDHINT_B:[0-9]+]] +// CHECK: store double 1.000000e+00, ptr addrspace(1) %[[PTR_A:.*]], align 8{{.*}}, !spirv.DecorationCacheControlINTEL ![[STHINT_A:[0-9]+]] +// CHECK: store double 1.000000e+00, ptr addrspace(1) %[[PTR_B:.*]], align 8{{.*}}, !spirv.DecorationCacheControlINTEL ![[STHINT_B:[0-9]+]] +// CHECK: load double, ptr addrspace(1) %[[PTR_A]], align 8{{.*}}, !spirv.DecorationCacheControlINTEL ![[LDHINT_A:[0-9]+]] +// CHECK: load double, ptr addrspace(1) %[[PTR_B]], align 8{{.*}}, !spirv.DecorationCacheControlINTEL ![[LDHINT_B:[0-9]+]] // CHECK: ret void -// CHECK: [[WHINT]] = !{[[WHINT1:.*]], [[WHINT2:.*]], [[WHINT3:.*]], [[WHINT4:.*]], i32 1} -// CHECK: [[WHINT1]] = !{i32 6443, i32 3, i32 3} -// CHECK: [[WHINT2]] = !{i32 6443, i32 0, i32 1} -// CHECK: [[WHINT3]] = !{i32 6443, i32 1, i32 2} -// CHECK: [[WHINT4]] = !{i32 6443, i32 2, i32 2} - -// CHECK: [[RHINT]] = !{[[RHINT1:.*]], [[RHINT2:.*]], [[RHINT3:.*]], i32 1} -// CHECK: [[RHINT1]] = !{i32 6442, i32 1, i32 0} -// CHECK: [[RHINT2]] = !{i32 6442, i32 2, i32 0} -// CHECK: [[RHINT3]] = !{i32 6442, i32 0, i32 1} - -// CHECK: [[RASSERT]] = !{[[RASSERT1:.*]], [[RASSERT2:.*]], [[RASSERT3:.*]], i32 1} -// CHECK: [[RASSERT1]] = !{i32 6442, i32 1, i32 3} -// CHECK: [[RASSERT2]] = !{i32 6442, i32 2, i32 3} -// CHECK: [[RASSERT3]] = !{i32 6442, i32 0, i32 4} - -// CHECK: [[RWHINT]] = !{[[RWHINT1:.*]], [[RWHINT2:.*]], [[RWHINT3:.*]], i32 1} -// CHECK: [[RWHINT1]] = !{i32 6442, i32 2, i32 1} -// CHECK: [[RWHINT2]] = !{i32 6442, i32 3, i32 4} -// CHECK: [[RWHINT3]] = !{i32 6443, i32 3, i32 1} - -// CHECK: [[STHINT_A]] = !{[[STHINT_A1:.*]], [[STHINT_A2:.*]], [[STHINT_A3:.*]], i32 1} -// CHECK: [[STHINT_A1]] = !{i32 6443, i32 0, i32 0} -// CHECK: [[STHINT_A2]] = !{i32 6443, i32 1, i32 0} -// CHECK: [[STHINT_A3]] = !{i32 6443, i32 2, i32 0} - -// CHECK: [[STHINT_B]] = !{[[STHINT_A2]], [[STHINT_A3]], [[STHINT_B1:.*]], i32 1} -// CHECK: [[STHINT_B1]] = !{i32 6443, i32 0, i32 2} - -// CHECK: [[LDHINT_A]] = !{[[RHINT1]], [[RHINT2]], [[RHINT3]], i32 0} -// CHECK: [[LDHINT_B]] = !{[[LDHINT_B1:.*]], [[RWHINT1]], [[LDHINT_B2:.*]], i32 0} -// CHECK: [[LDHINT_B1]] = !{i32 6442, i32 1, i32 1} -// CHECK: [[LDHINT_B2]] = !{i32 6442, i32 0, i32 2} +// CHECK: [[WHINT]] = !{[[WHINT1:.*]], [[WHINT2:.*]], [[WHINT3:.*]], [[WHINT4:.*]]} +// CHECK: [[WHINT1]] = !{i32 6443, i32 3, i32 3, i32 1} +// CHECK: [[WHINT2]] = !{i32 6443, i32 0, i32 1, i32 1} +// CHECK: [[WHINT3]] = !{i32 6443, i32 1, i32 2, i32 1} +// CHECK: [[WHINT4]] = !{i32 6443, i32 2, i32 2, i32 1} + +// CHECK: [[RHINT]] = !{[[RHINT1:.*]], [[RHINT2:.*]], [[RHINT3:.*]]} +// CHECK: [[RHINT1]] = !{i32 6442, i32 1, i32 0, i32 1} +// CHECK: [[RHINT2]] = !{i32 6442, i32 2, i32 0, i32 1} +// CHECK: [[RHINT3]] = !{i32 6442, i32 0, i32 1, i32 1} + +// CHECK: [[RASSERT]] = !{[[RASSERT1:.*]], [[RASSERT2:.*]], [[RASSERT3:.*]]} +// CHECK: [[RASSERT1]] = !{i32 6442, i32 1, i32 3, i32 1} +// CHECK: [[RASSERT2]] = !{i32 6442, i32 2, i32 3, i32 1} +// CHECK: [[RASSERT3]] = !{i32 6442, i32 0, i32 4, i32 1} + +// CHECK: [[RWHINT]] = !{[[RWHINT1:.*]], [[RWHINT2:.*]], [[RWHINT3:.*]]} +// CHECK: [[RWHINT1]] = !{i32 6442, i32 2, i32 1, i32 1} +// CHECK: [[RWHINT2]] = !{i32 6442, i32 3, i32 4, i32 1} +// CHECK: [[RWHINT3]] = !{i32 6443, i32 3, i32 1, i32 1} + +// CHECK: [[STHINT_A]] = !{[[STHINT_A1:.*]], [[STHINT_A2:.*]], [[STHINT_A3:.*]]} +// CHECK: [[STHINT_A1]] = !{i32 6443, i32 0, i32 0, i32 1} +// CHECK: [[STHINT_A2]] = !{i32 6443, i32 1, i32 0, i32 1} +// CHECK: [[STHINT_A3]] = !{i32 6443, i32 2, i32 0, i32 1} + +// CHECK: [[STHINT_B]] = !{[[STHINT_A2]], [[STHINT_A3]], [[STHINT_B1:.*]]} +// CHECK: [[STHINT_B1]] = !{i32 6443, i32 0, i32 2, i32 1} + +// CHECK: [[LDHINT_A]] = !{[[LDHINT_A1:.*]], [[LDHINT_A2:.*]], [[LDHINT_A3:.*]]} +// CHECK: [[LDHINT_A1]] = !{i32 6442, i32 1, i32 0, i32 0} +// CHECK: [[LDHINT_A2]] = !{i32 6442, i32 2, i32 0, i32 0} +// CHECK: [[LDHINT_A3]] = !{i32 6442, i32 0, i32 1, i32 0} + +// CHECK: [[LDHINT_B]] = !{[[LDHINT_B1:.*]], [[LDHINT_B2:.*]], [[LDHINT_B3:.*]]} +// CHECK: [[LDHINT_B1]] = !{i32 6442, i32 1, i32 1, i32 0} +// CHECK: [[LDHINT_B2]] = !{i32 6442, i32 2, i32 1, i32 0} +// CHECK: [[LDHINT_B3]] = !{i32 6442, i32 0, i32 2, i32 0}