[llvm] [SPIRV] Change how to detect OpenCL/Vulkan Env and update tests accordingly. (PR #129689)

Marcos Maronas via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 23 04:01:45 PDT 2025


https://github.com/maarquitos14 updated https://github.com/llvm/llvm-project/pull/129689

>From c956e4b77178fec6a2d45639f995ce56ea380beb Mon Sep 17 00:00:00 2001
From: "marcos.maronas" <mmaronas at smtp.igk.intel.com>
Date: Wed, 16 Apr 2025 16:56:23 +0200
Subject: [PATCH 1/3] Dissociate Logical/Physical from OpenCL/Vulkan env.

---
 llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp     |  6 ++-
 llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp   |  7 ++-
 llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp |  5 +-
 .../Target/SPIRV/SPIRVInstructionSelector.cpp | 40 ++++++++-------
 llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp | 51 ++++++++++++++++---
 .../Target/SPIRV/SPIRVPrepareFunctions.cpp    | 14 ++++-
 llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp      | 18 +++++--
 llvm/lib/Target/SPIRV/SPIRVSubtarget.h        | 29 ++++++++---
 llvm/lib/Target/SPIRV/SPIRVTargetMachine.cpp  |  9 +++-
 9 files changed, 138 insertions(+), 41 deletions(-)

diff --git a/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp b/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp
index f17c8a8fac14b..4780402f0a89c 100644
--- a/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp
@@ -532,7 +532,11 @@ void SPIRVAsmPrinter::outputExecutionMode(const Module &M) {
       Inst.addOperand(MCOperand::createImm(TypeCode));
       outputMCInst(Inst);
     }
-    if (ST->isOpenCLEnv() && !M.getNamedMetadata("spirv.ExecutionMode") &&
+    // FIXME: At the moment, `isOpenCLEnv()` is not precise enough. This is
+    // because the Triple is not always precise enough. For now, we'll rely
+    // instead on `isLogicalSPIRV()`, but this should be changed when
+    // `isOpenCLEnv()` is precise enough.
+    if (!ST->isLogicalSPIRV() && !M.getNamedMetadata("spirv.ExecutionMode") &&
         !M.getNamedMetadata("opencl.enable.FP_CONTRACT")) {
       MCInst Inst;
       Inst.setOpcode(SPIRV::OpExecutionMode);
diff --git a/llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp b/llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp
index 5ec8c22dbf473..52fc54a9d392a 100644
--- a/llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp
@@ -267,7 +267,12 @@ static SPIRVType *getArgSPIRVType(const Function &F, unsigned ArgIdx,
 
 static SPIRV::ExecutionModel::ExecutionModel
 getExecutionModel(const SPIRVSubtarget &STI, const Function &F) {
-  if (STI.isOpenCLEnv())
+  // FIXME: At the moment, there's a possibility that both `isOpenCLEnv()` and
+  // `isVulkanEnv()` return true. This is because the Triple is not always
+  // precise enough. For now, we'll rely instead on `isLogicalSPIRV()`, but this
+  // should be changed when `isOpenCLEnv()` and `isVulkanEnv()` cannot be true
+  // at the same time.
+  if (!STI.isLogicalSPIRV())
     return SPIRV::ExecutionModel::Kernel;
 
   auto attribute = F.getFnAttribute("hlsl.shader");
diff --git a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
index 4ce316ea32e1c..71fbe61571e96 100644
--- a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
@@ -767,7 +767,10 @@ Register SPIRVGlobalRegistry::buildGlobalVariable(
   // TODO: maybe move to GenerateDecorations pass.
   const SPIRVSubtarget &ST =
       cast<SPIRVSubtarget>(MIRBuilder.getMF().getSubtarget());
-  if (IsConst && ST.isOpenCLEnv())
+  // FIXME: Constant requires Kernel Capabilities, so we only emit it if we are
+  // in OpenCL env. However, that is not good enough at the moment, so we use
+  // `!isLogicalSPIRV()` instead.
+  if (IsConst && !ST.isLogicalSPIRV())
     buildOpDecorate(Reg, MIRBuilder, SPIRV::Decoration::Constant, {});
 
   if (GVar && GVar->getAlign().valueOrOne().value() != 1) {
diff --git a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
index 79f6b43f3aded..39cdf04b61345 100644
--- a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
@@ -862,7 +862,7 @@ bool SPIRVInstructionSelector::spvSelect(Register ResVReg,
                 .addUse(GV);
         return MIB.constrainAllUses(TII, TRI, RBI) &&
                BuildMI(BB, I, I.getDebugLoc(),
-                       TII.get(STI.isVulkanEnv()
+                       TII.get(STI.isLogicalSPIRV()
                                    ? SPIRV::OpInBoundsAccessChain
                                    : SPIRV::OpInBoundsPtrAccessChain))
                    .addDef(ResVReg)
@@ -1036,7 +1036,7 @@ bool SPIRVInstructionSelector::selectUnOp(Register ResVReg,
                                           const SPIRVType *ResType,
                                           MachineInstr &I,
                                           unsigned Opcode) const {
-  if (STI.isOpenCLEnv() && I.getOperand(1).isReg()) {
+  if (!STI.isLogicalSPIRV() && I.getOperand(1).isReg()) {
     Register SrcReg = I.getOperand(1).getReg();
     bool IsGV = false;
     for (MachineRegisterInfo::def_instr_iterator DefIt =
@@ -2069,7 +2069,7 @@ bool SPIRVInstructionSelector::selectDot4AddPackedExpansion(
   auto ExtractOp =
       Signed ? SPIRV::OpBitFieldSExtract : SPIRV::OpBitFieldUExtract;
 
-  bool ZeroAsNull = STI.isOpenCLEnv();
+  bool ZeroAsNull = !STI.isLogicalSPIRV();
   // Extract the i8 element, multiply and add it to the accumulator
   for (unsigned i = 0; i < 4; i++) {
     // A[i]
@@ -2209,7 +2209,7 @@ bool SPIRVInstructionSelector::selectWaveOpInst(Register ResVReg,
                  .addDef(ResVReg)
                  .addUse(GR.getSPIRVTypeID(ResType))
                  .addUse(GR.getOrCreateConstInt(SPIRV::Scope::Subgroup, I,
-                                                IntTy, TII, STI.isOpenCLEnv()));
+                                                IntTy, TII, !STI.isLogicalSPIRV()));
 
   for (unsigned J = 2; J < I.getNumOperands(); J++) {
     BMI.addUse(I.getOperand(J).getReg());
@@ -2233,7 +2233,7 @@ bool SPIRVInstructionSelector::selectWaveActiveCountBits(
                 .addDef(ResVReg)
                 .addUse(GR.getSPIRVTypeID(ResType))
                 .addUse(GR.getOrCreateConstInt(SPIRV::Scope::Subgroup, I, IntTy,
-                                               TII, STI.isOpenCLEnv()))
+                                               TII, !STI.isLogicalSPIRV()))
                 .addImm(SPIRV::GroupOperation::Reduce)
                 .addUse(BallotReg)
                 .constrainAllUses(TII, TRI, RBI);
@@ -2264,7 +2264,7 @@ bool SPIRVInstructionSelector::selectWaveReduceMax(Register ResVReg,
       .addDef(ResVReg)
       .addUse(GR.getSPIRVTypeID(ResType))
       .addUse(GR.getOrCreateConstInt(SPIRV::Scope::Subgroup, I, IntTy, TII,
-                                     STI.isOpenCLEnv()))
+                                     !STI.isLogicalSPIRV()))
       .addImm(SPIRV::GroupOperation::Reduce)
       .addUse(I.getOperand(2).getReg())
       .constrainAllUses(TII, TRI, RBI);
@@ -2291,7 +2291,7 @@ bool SPIRVInstructionSelector::selectWaveReduceSum(Register ResVReg,
       .addDef(ResVReg)
       .addUse(GR.getSPIRVTypeID(ResType))
       .addUse(GR.getOrCreateConstInt(SPIRV::Scope::Subgroup, I, IntTy, TII,
-                                     STI.isOpenCLEnv()))
+                                     !STI.isLogicalSPIRV()))
       .addImm(SPIRV::GroupOperation::Reduce)
       .addUse(I.getOperand(2).getReg());
 }
@@ -2513,7 +2513,7 @@ bool SPIRVInstructionSelector::selectFCmp(Register ResVReg,
 Register SPIRVInstructionSelector::buildZerosVal(const SPIRVType *ResType,
                                                  MachineInstr &I) const {
   // OpenCL uses nulls for Zero. In HLSL we don't use null constants.
-  bool ZeroAsNull = STI.isOpenCLEnv();
+  bool ZeroAsNull = !STI.isLogicalSPIRV();
   if (ResType->getOpcode() == SPIRV::OpTypeVector)
     return GR.getOrCreateConstVector(0UL, I, ResType, TII, ZeroAsNull);
   return GR.getOrCreateConstInt(0, I, ResType, TII, ZeroAsNull);
@@ -2522,7 +2522,7 @@ Register SPIRVInstructionSelector::buildZerosVal(const SPIRVType *ResType,
 Register SPIRVInstructionSelector::buildZerosValF(const SPIRVType *ResType,
                                                   MachineInstr &I) const {
   // OpenCL uses nulls for Zero. In HLSL we don't use null constants.
-  bool ZeroAsNull = STI.isOpenCLEnv();
+  bool ZeroAsNull = !STI.isLogicalSPIRV();
   APFloat VZero = getZeroFP(GR.getTypeForSPIRVType(ResType));
   if (ResType->getOpcode() == SPIRV::OpTypeVector)
     return GR.getOrCreateConstVector(VZero, I, ResType, TII, ZeroAsNull);
@@ -2532,7 +2532,7 @@ Register SPIRVInstructionSelector::buildZerosValF(const SPIRVType *ResType,
 Register SPIRVInstructionSelector::buildOnesValF(const SPIRVType *ResType,
                                                  MachineInstr &I) const {
   // OpenCL uses nulls for Zero. In HLSL we don't use null constants.
-  bool ZeroAsNull = STI.isOpenCLEnv();
+  bool ZeroAsNull = !STI.isLogicalSPIRV();
   APFloat VOne = getOneFP(GR.getTypeForSPIRVType(ResType));
   if (ResType->getOpcode() == SPIRV::OpTypeVector)
     return GR.getOrCreateConstVector(VOne, I, ResType, TII, ZeroAsNull);
@@ -2720,10 +2720,10 @@ bool SPIRVInstructionSelector::selectConst(Register ResVReg,
     Reg = GR.getOrCreateConstNullPtr(MIRBuilder, ResType);
   } else if (Opcode == TargetOpcode::G_FCONSTANT) {
     Reg = GR.getOrCreateConstFP(I.getOperand(1).getFPImm()->getValue(), I,
-                                ResType, TII, STI.isOpenCLEnv());
+                                ResType, TII, !STI.isLogicalSPIRV());
   } else {
     Reg = GR.getOrCreateConstInt(I.getOperand(1).getCImm()->getZExtValue(), I,
-                                 ResType, TII, STI.isOpenCLEnv());
+                                 ResType, TII, !STI.isLogicalSPIRV());
   }
   return Reg == ResVReg ? true : BuildCOPY(ResVReg, Reg, I);
 }
@@ -2803,7 +2803,7 @@ bool SPIRVInstructionSelector::selectGEP(Register ResVReg,
   // OpAccessChain could be used for OpenCL, but the SPIRV-LLVM Translator only
   // relies on PtrAccessChain, so we'll try not to deviate. For Vulkan however,
   // we have to use Op[InBounds]AccessChain.
-  const unsigned Opcode = STI.isVulkanEnv()
+  const unsigned Opcode = STI.isLogicalSPIRV()
                               ? (IsGEPInBounds ? SPIRV::OpInBoundsAccessChain
                                                : SPIRV::OpAccessChain)
                               : (IsGEPInBounds ? SPIRV::OpInBoundsPtrAccessChain
@@ -3483,7 +3483,7 @@ bool SPIRVInstructionSelector::selectFirstBitSet64Overflow(
 
   // On odd component counts we need to handle one more component
   if (CurrentComponent != ComponentCount) {
-    bool ZeroAsNull = STI.isOpenCLEnv();
+    bool ZeroAsNull = !STI.isLogicalSPIRV();
     Register FinalElemReg = MRI->createVirtualRegister(GR.getRegClass(I64Type));
     Register ConstIntLastIdx = GR.getOrCreateConstInt(
         ComponentCount - 1, I, BaseType, TII, ZeroAsNull);
@@ -3513,7 +3513,7 @@ bool SPIRVInstructionSelector::selectFirstBitSet64(
     Register SrcReg, unsigned BitSetOpcode, bool SwapPrimarySide) const {
   unsigned ComponentCount = GR.getScalarOrVectorComponentCount(ResType);
   SPIRVType *BaseType = GR.retrieveScalarOrVectorIntType(ResType);
-  bool ZeroAsNull = STI.isOpenCLEnv();
+  bool ZeroAsNull = !STI.isLogicalSPIRV();
   Register ConstIntZero =
       GR.getOrCreateConstInt(0, I, BaseType, TII, ZeroAsNull);
   Register ConstIntOne =
@@ -3715,7 +3715,10 @@ bool SPIRVInstructionSelector::selectAllocaArray(Register ResVReg,
                  .addUse(GR.getSPIRVTypeID(ResType))
                  .addUse(I.getOperand(2).getReg())
                  .constrainAllUses(TII, TRI, RBI);
-  if (!STI.isVulkanEnv()) {
+  // FIXME: Alignment requires Kernel Capabilities, so we only emit it if we are
+  // in OpenCL env. However, that is not good enough at the moment, so we use
+  // `!isLogicalSPIRV()` instead.
+  if (!STI.isLogicalSPIRV()) {
     unsigned Alignment = I.getOperand(3).getImm();
     buildOpDecorate(ResVReg, I, TII, SPIRV::Decoration::Alignment, {Alignment});
   }
@@ -3734,7 +3737,10 @@ bool SPIRVInstructionSelector::selectFrameIndex(Register ResVReg,
                  .addUse(GR.getSPIRVTypeID(ResType))
                  .addImm(static_cast<uint32_t>(SPIRV::StorageClass::Function))
                  .constrainAllUses(TII, TRI, RBI);
-  if (!STI.isVulkanEnv()) {
+  // FIXME: Alignment requires Kernel Capabilities, so we only emit it if we are
+  // in OpenCL env. However, that is not good enough at the moment, so we use
+  // `!isLogicalSPIRV()` instead.
+  if (!STI.isLogicalSPIRV()) {
     unsigned Alignment = I.getOperand(2).getImm();
     buildOpDecorate(ResVReg, *It, TII, SPIRV::Decoration::Alignment,
                     {Alignment});
diff --git a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
index 6e1c41d9f20cb..2aa99ec7a33ef 100644
--- a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
@@ -68,7 +68,12 @@ getSymbolicOperandRequirements(SPIRV::OperandCategory::OperandCategory Category,
                                SPIRV::RequirementHandler &Reqs) {
   // A set of capabilities to avoid if there is another option.
   AvoidCapabilitiesSet AvoidCaps;
-  if (ST.isOpenCLEnv())
+  // FIXME: At the moment, there's a possibility that both `isOpenCLEnv()` and
+  // `isVulkanEnv()` return true. This is because the Triple is not always
+  // precise enough. For now, we'll rely instead on `isLogicalSPIRV()`, but this
+  // should be changed when `isOpenCLEnv()` and `isVulkanEnv()` cannot be true
+  // at the same time.
+  if (!ST.isLogicalSPIRV())
     AvoidCaps.S.insert(SPIRV::Capability::Shader);
 
   VersionTuple ReqMinVer = getSymbolicOperandMinVersion(Category, i);
@@ -144,7 +149,12 @@ void SPIRVModuleAnalysis::setBaseInfo(const Module &M) {
         static_cast<SPIRV::MemoryModel::MemoryModel>(getMetadataUInt(MemMD, 1));
   } else {
     // TODO: Add support for VulkanMemoryModel.
-    MAI.Mem = ST->isOpenCLEnv() ? SPIRV::MemoryModel::OpenCL
+    // FIXME: At the moment, there's a possibility that both `isOpenCLEnv()` and
+    // `isVulkanEnv()` return true. This is because the Triple is not always
+    // precise enough. For now, we'll rely instead on `isLogicalSPIRV()`, but this
+    // should be changed when `isOpenCLEnv()` and `isVulkanEnv()` cannot be true
+    // at the same time.
+    MAI.Mem = !ST->isLogicalSPIRV() ? SPIRV::MemoryModel::OpenCL
                                 : SPIRV::MemoryModel::GLSL450;
     if (MAI.Mem == SPIRV::MemoryModel::OpenCL) {
       unsigned PtrSize = ST->getPointerSize();
@@ -203,7 +213,12 @@ void SPIRVModuleAnalysis::setBaseInfo(const Module &M) {
   MAI.Reqs.getAndAddRequirements(SPIRV::OperandCategory::AddressingModelOperand,
                                  MAI.Addr, *ST);
 
-  if (ST->isOpenCLEnv()) {
+  // FIXME: At the moment, there's a possibility that both `isOpenCLEnv()` and
+  // `isVulkanEnv()` return true. This is because the Triple is not always
+  // precise enough. For now, we'll rely instead on `isLogicalSPIRV()`, but this
+  // should be changed when `isOpenCLEnv()` and `isVulkanEnv()` cannot be true
+  // at the same time.
+  if (!ST->isLogicalSPIRV()) {
     // TODO: check if it's required by default.
     MAI.ExtInstSetMap[static_cast<unsigned>(
         SPIRV::InstructionSet::OpenCL_std)] = MAI.getNextIDRegister();
@@ -804,12 +819,17 @@ void RequirementHandler::initAvailableCapabilities(const SPIRVSubtarget &ST) {
     addAvailableCaps(EnabledCapabilities);
   }
 
-  if (ST.isOpenCLEnv()) {
+  // FIXME: At the moment, there's a possibility that both `isOpenCLEnv()` and
+  // `isVulkanEnv()` return true. This is because the Triple is not always
+  // precise enough. For now, we'll rely instead on `isLogicalSPIRV`, but this
+  // should be changed when `isOpenCLEnv()` and `isVulkanEnv()` cannot be true
+  // at the same time.
+  if (!ST.isLogicalSPIRV()) {
     initAvailableCapabilitiesForOpenCL(ST);
     return;
   }
 
-  if (ST.isVulkanEnv()) {
+  if (ST.isLogicalSPIRV()) {
     initAvailableCapabilitiesForVulkan(ST);
     return;
   }
@@ -969,7 +989,12 @@ static void addOpTypeImageReqs(const MachineInstr &MI,
   }
 
   // Has optional access qualifier.
-  if (ST.isOpenCLEnv()) {
+  // FIXME: ImageBasic/ImageReadWrite capabilities require Kernel capability.
+  // However, for now, both `isVulkanEnv()` and `isOpenCLEnv()` can return
+  // true under some circumstances. Instead, we're using `isLogicalSPIRV()`,
+  // but we should change this when `isVulkanEnv()` and `isOpenCLEnv()` are
+  // precise enough.
+  if (!ST.isLogicalSPIRV()) {
     if (MI.getNumOperands() > 8 &&
         MI.getOperand(8).getImm() == SPIRV::AccessQualifier::ReadWrite)
       Reqs.addRequirements(SPIRV::Capability::ImageReadWrite);
@@ -1267,7 +1292,12 @@ void addInstrRequirements(const MachineInstr &MI,
                                ST);
     // If it's a type of pointer to float16 targeting OpenCL, add Float16Buffer
     // capability.
-    if (!ST.isOpenCLEnv())
+    // FIXME: Float16Buffer capability requires Kernel capability. However,
+    // for now, both `isVulkanEnv()` and `isOpenCLEnv()` can return true under
+    // some circumstances. Instead, we're using `isLogicalSPIRV()`, but we
+    // should change this when `isVulkanEnv()` and `isOpenCLEnv()` are precise
+    // enough.
+    if (ST.isLogicalSPIRV())
       break;
     assert(MI.getOperand(2).isReg());
     const MachineRegisterInfo &MRI = MI.getMF()->getRegInfo();
@@ -1342,7 +1372,12 @@ void addInstrRequirements(const MachineInstr &MI,
     addOpTypeImageReqs(MI, Reqs, ST);
     break;
   case SPIRV::OpTypeSampler:
-    if (!ST.isVulkanEnv()) {
+    // FIXME: ImageBasic capability requires Kernel capability. However, for
+    // now, both `isVulkanEnv()` and `isOpenCLEnv()` can return true under
+    // some circumstances. Instead, we're using `isLogicalSPIRV()`, but
+    // we should change this when `isVulkanEnv()` and `isOpenCLEnv()` are
+    // precise enough.
+    if (!ST.isLogicalSPIRV()) {
       Reqs.addCapability(SPIRV::Capability::ImageBasic);
     }
     break;
diff --git a/llvm/lib/Target/SPIRV/SPIRVPrepareFunctions.cpp b/llvm/lib/Target/SPIRV/SPIRVPrepareFunctions.cpp
index bd039871dec44..d2c51c80c1507 100644
--- a/llvm/lib/Target/SPIRV/SPIRVPrepareFunctions.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVPrepareFunctions.cpp
@@ -407,13 +407,23 @@ bool SPIRVPrepareFunctions::substituteIntrinsicCalls(Function *F) {
         Changed = true;
         break;
       case Intrinsic::lifetime_start:
-        if (STI.isOpenCLEnv()) {
+        // FIXME: OpLifetimeStart requires Kernel capability. However, for now,
+        // both `isVulkanEnv()` and `isOpenCLEnv()` can return true under some
+        // circumstances. Instead, we're using `isLogicalSPIRV()`, but we
+        // should change this when `isVulkanEnv()` and `isOpenCLEnv()` are
+        // precise enough.
+        if (!STI.isLogicalSPIRV()) {
           Changed |= toSpvOverloadedIntrinsic(
               II, Intrinsic::SPVIntrinsics::spv_lifetime_start, {1});
         }
         break;
       case Intrinsic::lifetime_end:
-        if (STI.isOpenCLEnv()) {
+        // FIXME: OpLifetimeStop requires Kernel capability. However, for now,
+        // both `isVulkanEnv()` and `isOpenCLEnv()` can return true under some
+        // circumstances. Instead, we're using `isLogicalSPIRV()`, but we
+        // should change this when `isVulkanEnv()` and `isOpenCLEnv()` are
+        // precise enough.
+        if (!STI.isLogicalSPIRV()) {
           Changed |= toSpvOverloadedIntrinsic(
               II, Intrinsic::SPVIntrinsics::spv_lifetime_end, {1});
         }
diff --git a/llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp b/llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp
index 236a1272bcebb..f6457f7fb93f1 100644
--- a/llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp
@@ -113,7 +113,11 @@ bool SPIRVSubtarget::canUseExtInstSet(
 
 SPIRV::InstructionSet::InstructionSet
 SPIRVSubtarget::getPreferredInstructionSet() const {
-  if (isOpenCLEnv())
+  // FIXME: For now, both `isVulkanEnv()` and `isOpenCLEnv()` can return true
+  // under some circumstances. Instead, we're using `isLogicalSPIRV()`, but we
+  // should change this when `isVulkanEnv()` and `isOpenCLEnv()` are precise
+  // enough.
+  if (!isLogicalSPIRV())
     return SPIRV::InstructionSet::OpenCL_std;
   else
     return SPIRV::InstructionSet::GLSL_std_450;
@@ -124,7 +128,11 @@ bool SPIRVSubtarget::isAtLeastSPIRVVer(VersionTuple VerToCompareTo) const {
 }
 
 bool SPIRVSubtarget::isAtLeastOpenCLVer(VersionTuple VerToCompareTo) const {
-  if (!isOpenCLEnv())
+  // FIXME: For now, both `isVulkanEnv()` and `isOpenCLEnv()` can return true
+  // under some circumstances. Instead, we're using `isLogicalSPIRV()`, but we
+  // should change this when `isVulkanEnv()` and `isOpenCLEnv()` are precise
+  // enough.
+  if (isLogicalSPIRV())
     return false;
   return isAtLeastVer(OpenCLVersion, VerToCompareTo);
 }
@@ -147,7 +155,11 @@ void SPIRVSubtarget::accountForAMDShaderTrinaryMinmax() {
 // Must have called initAvailableExtensions first.
 void SPIRVSubtarget::initAvailableExtInstSets() {
   AvailableExtInstSets.clear();
-  if (!isOpenCLEnv())
+  // FIXME: For now, both `isVulkanEnv()` and `isOpenCLEnv()` can return true
+  // under some circumstances. Instead, we're using `isLogicalSPIRV()`, but we
+  // should change this when `isVulkanEnv()` and `isOpenCLEnv()` are precise
+  // enough.
+  if (isLogicalSPIRV())
     AvailableExtInstSets.insert(SPIRV::InstructionSet::GLSL_std_450);
   else
     AvailableExtInstSets.insert(SPIRV::InstructionSet::OpenCL_std);
diff --git a/llvm/lib/Target/SPIRV/SPIRVSubtarget.h b/llvm/lib/Target/SPIRV/SPIRVSubtarget.h
index e4484f6508b6b..3910fef4b8cbf 100644
--- a/llvm/lib/Target/SPIRV/SPIRVSubtarget.h
+++ b/llvm/lib/Target/SPIRV/SPIRVSubtarget.h
@@ -36,6 +36,8 @@ class StringRef;
 class SPIRVTargetMachine;
 
 class SPIRVSubtarget : public SPIRVGenSubtargetInfo {
+  // Enum for the SPIR-V environment: OpenCL, Vulkan or Unkwnown.
+  enum SPIRVEnvType { OpenCL, Vulkan, Unknown };
 private:
   const unsigned PointerSize;
   VersionTuple SPIRVVersion;
@@ -49,6 +51,7 @@ class SPIRVSubtarget : public SPIRVGenSubtargetInfo {
   SPIRVFrameLowering FrameLowering;
   SPIRVTargetLowering TLInfo;
   Triple TargetTriple;
+  SPIRVEnvType Env;
 
   // GlobalISel related APIs.
   std::unique_ptr<CallLowering> CallLoweringInfo;
@@ -78,14 +81,28 @@ class SPIRVSubtarget : public SPIRVGenSubtargetInfo {
   unsigned getPointerSize() const { return PointerSize; }
   unsigned getBound() const { return GR->getBound(); }
   bool canDirectlyComparePointers() const;
-  // TODO: this environment is not implemented in Triple, we need to decide
-  // how to standardize its support. For now, let's assume SPIR-V with physical
-  // addressing is OpenCL, and Logical addressing is Vulkan.
+  SPIRVEnvType getEnv() const {
+    if (TargetTriple.getOS() == Triple::Vulkan)
+      return Vulkan;
+    if (TargetTriple.getEnvironment() == Triple::OpenCL)
+      return OpenCL;
+    return Unknown;
+  }
+  // FIXME: For now, we rely only on the triple to determine the environment.
+  // However, a lot of frontends emit unknown OS or environment, which makes it
+  // difficult to determine the environment. We should consider adding other
+  // methods. For now, we will return true for both OpenCL and Unknown env.
   bool isOpenCLEnv() const {
-    return TargetTriple.getArch() == Triple::spirv32 ||
-           TargetTriple.getArch() == Triple::spirv64;
+    return getEnv() == OpenCL || getEnv() == Unknown;
+  }
+  // FIXME: For now, we rely only on the triple to determine the environment.
+  // However, a lot of frontends emit unknown OS or environment, which makes it
+  // difficult to determine the environment. We should consider adding other
+  // methods. For now, we will return true for both Vulkan and Unknown env.
+  bool isVulkanEnv() const {
+    return getEnv() == Vulkan || getEnv() == Unknown;
   }
-  bool isVulkanEnv() const { return TargetTriple.getArch() == Triple::spirv; }
+  bool isLogicalSPIRV() const { return TargetTriple.getArch() == Triple::spirv; }
   const std::string &getTargetTripleAsStr() const { return TargetTriple.str(); }
   VersionTuple getSPIRVVersion() const { return SPIRVVersion; };
   bool isAtLeastSPIRVVer(VersionTuple VerToCompareTo) const;
diff --git a/llvm/lib/Target/SPIRV/SPIRVTargetMachine.cpp b/llvm/lib/Target/SPIRV/SPIRVTargetMachine.cpp
index 68286737b972f..652ba85099728 100644
--- a/llvm/lib/Target/SPIRV/SPIRVTargetMachine.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVTargetMachine.cpp
@@ -189,7 +189,12 @@ TargetPassConfig *SPIRVTargetMachine::createPassConfig(PassManagerBase &PM) {
 void SPIRVPassConfig::addIRPasses() {
   TargetPassConfig::addIRPasses();
 
-  if (TM.getSubtargetImpl()->isVulkanEnv()) {
+  // FIXME: At the moment, there's a possibility that both `isOpenCLEnv()` and
+  // `isVulkanEnv()` return true. This is because the Triple is not always
+  // precise enough. For now, we'll rely instead on `isLogicalSPIRV`, but this
+  // should be changed when `isOpenCLEnv()` and `isVulkanEnv()` cannot be true
+  // at the same time.
+  if (TM.getSubtargetImpl()->isLogicalSPIRV()) {
     // 1.  Simplify loop for subsequent transformations. After this steps, loops
     // have the following properties:
     //  - loops have a single entry edge (pre-header to loop header).
@@ -221,7 +226,7 @@ void SPIRVPassConfig::addIRPasses() {
 
 void SPIRVPassConfig::addISelPrepare() {
   addPass(createSPIRVEmitIntrinsicsPass(&getTM<SPIRVTargetMachine>()));
-  if (TM.getSubtargetImpl()->isVulkanEnv())
+  if (TM.getSubtargetImpl()->isLogicalSPIRV())
     addPass(createSPIRVLegalizePointerCastPass(&getTM<SPIRVTargetMachine>()));
   TargetPassConfig::addISelPrepare();
 }

>From f87ac688203e43429aa4b27493a5221f01ed9949 Mon Sep 17 00:00:00 2001
From: Marcos Maronas <marcos.maronas at intel.com>
Date: Wed, 16 Apr 2025 17:44:06 +0200
Subject: [PATCH 2/3] Extra change to complete dissociation.

---
 llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
index 2aa99ec7a33ef..e8b5d647b29f2 100644
--- a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
@@ -185,7 +185,11 @@ void SPIRVModuleAnalysis::setBaseInfo(const Module &M) {
     // OpenCL 1.0 by default for the OpenCL environment to avoid puzzling
     // run-times with Unknown/0.0 version output. For a reference, LLVM-SPIRV
     // Translator avoids potential issues with run-times in a similar manner.
-    if (ST->isOpenCLEnv()) {
+    // FIXME: At the moment, `isOpenCLEnv()` is not precise enough. This is
+    // because the Triple is not always precise enough. For now, we'll rely
+    // instead on `isLogicalSPIRV()`, but this should be changed when
+    // `isOpenCLEnv()` is precise enough.
+    if (!ST->isLogicalSPIRV()) {
       MAI.SrcLang = SPIRV::SourceLanguage::OpenCL_CPP;
       MAI.SrcLangVersion = 100000;
     } else {

>From eef5045e98d4fccbeb8f91edd87205a0c165cfbf Mon Sep 17 00:00:00 2001
From: Marcos Maronas <marcos.maronas at intel.com>
Date: Wed, 23 Apr 2025 12:58:01 +0200
Subject: [PATCH 3/3] Use entry points to help determine the env.

---
 llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp   | 34 +++++++++++++++--
 llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp | 20 +++++++---
 llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp      |  8 ++++
 llvm/lib/Target/SPIRV/SPIRVSubtarget.h        | 37 ++++++++-----------
 4 files changed, 69 insertions(+), 30 deletions(-)

diff --git a/llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp b/llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp
index 52fc54a9d392a..e6d056107d7d9 100644
--- a/llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp
@@ -272,14 +272,37 @@ getExecutionModel(const SPIRVSubtarget &STI, const Function &F) {
   // precise enough. For now, we'll rely instead on `isLogicalSPIRV()`, but this
   // should be changed when `isOpenCLEnv()` and `isVulkanEnv()` cannot be true
   // at the same time.
-  if (!STI.isLogicalSPIRV())
+  if (STI.isOpenCLEnv())
     return SPIRV::ExecutionModel::Kernel;
 
+  if (STI.isVulkanEnv()) {
+    auto attribute = F.getFnAttribute("hlsl.shader");
+    if (!attribute.isValid()) {
+      report_fatal_error(
+          "This entry point lacks mandatory hlsl.shader attribute.");
+    }
+
+    const auto value = attribute.getValueAsString();
+    if (value == "compute")
+      return SPIRV::ExecutionModel::GLCompute;
+
+    report_fatal_error(
+        "This HLSL entry point is not supported by this backend.");
+  }
+
+  assert(STI.getEnv() == Unknown);
+  // Can we rely on "hlsl.shader" attribute? Is it mandatory for Vulkan env? If
+  // so, we can set Env to Vulkan whenever we find it, and to OpenCL otherwise.
+
+  // We will now change the Env based on the attribute, so we need to strip
+  // `const` out of the ref to STI.
+  SPIRVSubtarget *NonConstSTI = const_cast<SPIRVSubtarget *>(&STI);
   auto attribute = F.getFnAttribute("hlsl.shader");
   if (!attribute.isValid()) {
-    report_fatal_error(
-        "This entry point lacks mandatory hlsl.shader attribute.");
+    NonConstSTI->setEnv(SPIRVSubtarget::OpenCL);
+    return SPIRV::ExecutionModel::Kernel;
   }
+  NonConstSTI->setEnv(SPIRVSubtarget::Vulkan);
 
   const auto value = attribute.getValueAsString();
   if (value == "compute")
@@ -444,6 +467,11 @@ bool SPIRVCallLowering::lowerFormalArguments(MachineIRBuilder &MIRBuilder,
 
   // Handle entry points and function linkage.
   if (isEntryPoint(F)) {
+    // EntryPoints can help us to determine the environment we're working on.
+    // Therefore, we need a non-const pointer to SPIRVSubtarget to update the
+    // environment if we need to.
+    const SPIRVSubtarget *ST =
+        static_cast<const SPIRVSubtarget *>(&MIRBuilder.getMF().getSubtarget());
     auto MIB = MIRBuilder.buildInstr(SPIRV::OpEntryPoint)
                    .addImm(static_cast<uint32_t>(getExecutionModel(*ST, F)))
                    .addUse(FuncVReg);
diff --git a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
index e8b5d647b29f2..10428d245af06 100644
--- a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
@@ -151,11 +151,11 @@ void SPIRVModuleAnalysis::setBaseInfo(const Module &M) {
     // TODO: Add support for VulkanMemoryModel.
     // FIXME: At the moment, there's a possibility that both `isOpenCLEnv()` and
     // `isVulkanEnv()` return true. This is because the Triple is not always
-    // precise enough. For now, we'll rely instead on `isLogicalSPIRV()`, but this
-    // should be changed when `isOpenCLEnv()` and `isVulkanEnv()` cannot be true
-    // at the same time.
+    // precise enough. For now, we'll rely instead on `isLogicalSPIRV()`, but
+    // this should be changed when `isOpenCLEnv()` and `isVulkanEnv()` cannot be
+    // true at the same time.
     MAI.Mem = !ST->isLogicalSPIRV() ? SPIRV::MemoryModel::OpenCL
-                                : SPIRV::MemoryModel::GLSL450;
+                                    : SPIRV::MemoryModel::GLSL450;
     if (MAI.Mem == SPIRV::MemoryModel::OpenCL) {
       unsigned PtrSize = ST->getPointerSize();
       MAI.Addr = PtrSize == 32   ? SPIRV::AddressingModel::Physical32
@@ -1808,7 +1808,11 @@ void addInstrRequirements(const MachineInstr &MI,
     // not allowed to produce
     // StorageImageReadWithoutFormat/StorageImageWriteWithoutFormat, see
     // https://github.com/KhronosGroup/SPIRV-Headers/issues/487
-    if (isImageTypeWithUnknownFormat(TypeDef) && !ST.isOpenCLEnv())
+
+    // FIXME: For now, `isOpenCLEnv()` is not precise enough. Instead, we're
+    // using `isLogicalSPIRV()`, but we should change this when `isOpenCLEnv()`
+    // is precise enough.
+    if (isImageTypeWithUnknownFormat(TypeDef) && ST.isLogicalSPIRV())
       Reqs.addCapability(SPIRV::Capability::StorageImageReadWithoutFormat);
     break;
   }
@@ -1821,7 +1825,11 @@ void addInstrRequirements(const MachineInstr &MI,
     // not allowed to produce
     // StorageImageReadWithoutFormat/StorageImageWriteWithoutFormat, see
     // https://github.com/KhronosGroup/SPIRV-Headers/issues/487
-    if (isImageTypeWithUnknownFormat(TypeDef) && !ST.isOpenCLEnv())
+
+    // FIXME: For now, `isOpenCLEnv()` is not precise enough. Instead, we're
+    // using `isLogicalSPIRV()`, but we should change this when `isOpenCLEnv()`
+    // is precise enough.
+    if (isImageTypeWithUnknownFormat(TypeDef) && ST.isLogicalSPIRV())
       Reqs.addCapability(SPIRV::Capability::StorageImageWriteWithoutFormat);
     break;
   }
diff --git a/llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp b/llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp
index f6457f7fb93f1..e76646e4fa7a7 100644
--- a/llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp
@@ -83,6 +83,14 @@ SPIRVSubtarget::SPIRVSubtarget(const Triple &TT, const std::string &CPU,
   }
   OpenCLVersion = VersionTuple(2, 2);
 
+  // Set the environment based on the target triple.
+  if (TargetTriple.getOS() == Triple::Vulkan)
+    Env = Vulkan;
+  else if (TargetTriple.getEnvironment() == Triple::OpenCL)
+    Env = OpenCL;
+  else
+    Env = Unknown;
+
   // The order of initialization is important.
   initAvailableExtensions(Extensions);
   initAvailableExtInstSets();
diff --git a/llvm/lib/Target/SPIRV/SPIRVSubtarget.h b/llvm/lib/Target/SPIRV/SPIRVSubtarget.h
index 3910fef4b8cbf..6101ed6c71b4b 100644
--- a/llvm/lib/Target/SPIRV/SPIRVSubtarget.h
+++ b/llvm/lib/Target/SPIRV/SPIRVSubtarget.h
@@ -36,8 +36,10 @@ class StringRef;
 class SPIRVTargetMachine;
 
 class SPIRVSubtarget : public SPIRVGenSubtargetInfo {
+public:
   // Enum for the SPIR-V environment: OpenCL, Vulkan or Unkwnown.
   enum SPIRVEnvType { OpenCL, Vulkan, Unknown };
+
 private:
   const unsigned PointerSize;
   VersionTuple SPIRVVersion;
@@ -81,28 +83,21 @@ class SPIRVSubtarget : public SPIRVGenSubtargetInfo {
   unsigned getPointerSize() const { return PointerSize; }
   unsigned getBound() const { return GR->getBound(); }
   bool canDirectlyComparePointers() const;
-  SPIRVEnvType getEnv() const {
-    if (TargetTriple.getOS() == Triple::Vulkan)
-      return Vulkan;
-    if (TargetTriple.getEnvironment() == Triple::OpenCL)
-      return OpenCL;
-    return Unknown;
-  }
-  // FIXME: For now, we rely only on the triple to determine the environment.
-  // However, a lot of frontends emit unknown OS or environment, which makes it
-  // difficult to determine the environment. We should consider adding other
-  // methods. For now, we will return true for both OpenCL and Unknown env.
-  bool isOpenCLEnv() const {
-    return getEnv() == OpenCL || getEnv() == Unknown;
-  }
-  // FIXME: For now, we rely only on the triple to determine the environment.
-  // However, a lot of frontends emit unknown OS or environment, which makes it
-  // difficult to determine the environment. We should consider adding other
-  // methods. For now, we will return true for both Vulkan and Unknown env.
-  bool isVulkanEnv() const {
-    return getEnv() == Vulkan || getEnv() == Unknown;
+  void setEnv(SPIRVEnvType E) {
+    assert(E != Unknown && "Unknown environment is not allowed");
+    assert(Env == Unknown && "Environment is already set");
+
+    Env = E;
   }
-  bool isLogicalSPIRV() const { return TargetTriple.getArch() == Triple::spirv; }
+  SPIRVEnvType getEnv() const { return Env; }
+  bool isOpenCLEnv() const { return getEnv() == OpenCL; }
+  bool isVulkanEnv() const { return getEnv() == Vulkan; }
+  // FIXME: This should check the triple arch instead, but a lot of places use
+  // this method now instead of `is[OpenCL/Vulkan]Env()`, and this is a
+  // shortcut to make sure `is[OpenCL/Vulkan]Env()` works as expected. When we
+  // change back all uses of `isLogicalSPIRV()` to `is[OpenCL/Vulkan]Env()`, we
+  // can implement this correctly again.
+  bool isLogicalSPIRV() const { return isVulkanEnv(); }
   const std::string &getTargetTripleAsStr() const { return TargetTriple.str(); }
   VersionTuple getSPIRVVersion() const { return SPIRVVersion; };
   bool isAtLeastSPIRVVer(VersionTuple VerToCompareTo) const;



More information about the llvm-commits mailing list