Skip to content
39 changes: 5 additions & 34 deletions llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -221,41 +221,17 @@ static SPIRVType *getArgSPIRVType(const Function &F, unsigned ArgIdx,

static SPIRV::ExecutionModel::ExecutionModel
getExecutionModel(const SPIRVSubtarget &STI, const Function &F) {
assert(STI.getEnv() != SPIRVSubtarget::Unknown &&
"Environment must be resolved before lowering entry points. ");
Comment thread
MrSidims marked this conversation as resolved.
Outdated

if (STI.isKernel())
return SPIRV::ExecutionModel::Kernel;

if (STI.isShader()) {
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;
if (value == "vertex")
return SPIRV::ExecutionModel::Vertex;
if (value == "pixel")
return SPIRV::ExecutionModel::Fragment;

report_fatal_error(
"This HLSL entry point is not supported by this backend.");
}

assert(STI.getEnv() == SPIRVSubtarget::Unknown);
// "hlsl.shader" attribute is mandatory for Vulkan, so we can set Env to
// Shader whenever we find it, and to Kernel 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()) {
NonConstSTI->setEnv(SPIRVSubtarget::Kernel);
return SPIRV::ExecutionModel::Kernel;
report_fatal_error(
"This entry point lacks mandatory hlsl.shader attribute.");
}
NonConstSTI->setEnv(SPIRVSubtarget::Shader);

const auto value = attribute.getValueAsString();
if (value == "compute")
Expand Down Expand Up @@ -432,11 +408,6 @@ 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);
Expand Down
7 changes: 7 additions & 0 deletions llvm/lib/Target/SPIRV/SPIRVPrepareFunctions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -656,6 +656,13 @@ bool SPIRVPrepareFunctions::removeAggregateTypesFromCalls(Function *F) {
}

bool SPIRVPrepareFunctions::runOnModule(Module &M) {
// Resolve the SPIR-V environment from module content before any
// function-level processing. This must happen before legalization so that
// isShader()/isKernel() return correct values.
const_cast<SPIRVTargetMachine &>(TM)
.getMutableSubtargetImpl()
->resolveEnvFromModule(M);

bool Changed = false;
for (Function &F : M) {
Changed |= substituteIntrinsicCalls(&F);
Expand Down
19 changes: 19 additions & 0 deletions llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,25 @@ void SPIRVSubtarget::initAvailableExtInstSets() {
accountForAMDShaderTrinaryMinmax();
}

void SPIRVSubtarget::resolveEnvFromModule(const Module &M) {
if (Env != Unknown)
return;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we care about the case where the same instance of the backend is used to compile multiple modules that may have different ENVs? I think it is safe to fail in that case because that is generally not supported by an LLVM module anyway. I just want to be sure we make an explicit decision.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should care, but probably it's a good idea to add an assertion to document in the code our expectations.


bool HasShaderAttr = false;
for (const Function &F : M) {
if (F.hasFnAttribute("hlsl.shader")) {
HasShaderAttr = true;
break;
}
}

Env = HasShaderAttr ? Shader : Kernel;

// Reinitialize Env-dependent state: ext inst sets and legalizer info.
initAvailableExtInstSets();
Legalizer = std::make_unique<SPIRVLegalizerInfo>(*this);
}

// Set available extensions after SPIRVSubtarget is created.
void SPIRVSubtarget::initAvailableExtensions(
const std::set<SPIRV::Extension::Extension> &AllowedExtIds) {
Expand Down
11 changes: 6 additions & 5 deletions llvm/lib/Target/SPIRV/SPIRVSubtarget.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "llvm/CodeGen/SelectionDAGTargetInfo.h"
#include "llvm/CodeGen/TargetSubtargetInfo.h"
#include "llvm/IR/DataLayout.h"
#include "llvm/IR/Module.h"
#include "llvm/Target/TargetMachine.h"
#include "llvm/TargetParser/Triple.h"

Expand Down Expand Up @@ -62,9 +63,6 @@ class SPIRVSubtarget : public SPIRVGenSubtargetInfo {
std::unique_ptr<InstructionSelector> InstSelector;
std::unique_ptr<InlineAsmLowering> InlineAsmInfo;

// TODO: Initialise the available extensions, extended instruction sets
// based on the environment settings.
void initAvailableExtInstSets();
void accountForAMDShaderTrinaryMinmax();

public:
Expand All @@ -76,6 +74,9 @@ class SPIRVSubtarget : public SPIRVGenSubtargetInfo {

void initAvailableExtensions(
const std::set<SPIRV::Extension::Extension> &AllowedExtIds);
void initAvailableExtInstSets();

void resolveEnvFromModule(const Module &M);

// Parses features string setting specified subtarget options.
// The definition of this function is auto generated by tblgen.
Expand All @@ -86,8 +87,8 @@ class SPIRVSubtarget : public SPIRVGenSubtargetInfo {
void setEnv(SPIRVEnvType E) {
if (E == Unknown)
report_fatal_error("Unknown environment is not allowed.");
if (Env != Unknown)
report_fatal_error("Environment is already set.");
if (Env != Unknown && Env != E)
report_fatal_error("Environment is already set to a different value.");

Env = E;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this function called anymore? I think the two calls to the function were removed, and we could simply remove it?

It creates a problem. If someone were to call it the instruction sets would not be reinitialized properly. If we keep it we need to fix that up.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I think about it more. Can you have this function call initAvailableExtInstSets and reinitialize the legalizer. Then both places that set Env should call this function. Having 1 place that sets Env could help if we find other things that need to be reset based on the env.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I've made resolveEnvFromModule calling setEnv and also moved initAvailableExtInstSets and set of legalizer info to the setEnv

Expand Down
2 changes: 2 additions & 0 deletions llvm/lib/Target/SPIRV/SPIRVTargetMachine.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ class SPIRVTargetMachine : public CodeGenTargetMachineImpl {
return &Subtarget;
}

SPIRVSubtarget *getMutableSubtargetImpl() { return &Subtarget; }

TargetTransformInfo getTargetTransformInfo(const Function &F) const override;

TargetPassConfig *createPassConfig(PassManagerBase &PM) override;
Expand Down
25 changes: 25 additions & 0 deletions llvm/test/CodeGen/SPIRV/is-shader-env.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
; RUN: llc -O0 -mtriple=spirv-unknown-unknown %s -o - | FileCheck %s
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv-unknown-unknown %s -o - -filetype=obj | spirv-val %}
Comment thread
MrSidims marked this conversation as resolved.
Outdated

; Regression test for https://github.com/llvm/llvm-project/issues/171898
; When triple is spirv-unknown-unknown and a non-entry-point function using
; wide vectors (e.g. <8 x i32>) appears before the entry point with
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is the use of the wide vector? @helper uses on 4xi32.

Copy link
Copy Markdown
Contributor Author

@MrSidims MrSidims Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ehhh, how I ended up with this test? Some odd script put in llvm-reduce script may be? Thanks for spotting!

; hlsl.shader attribute, the environment must be resolved early enough that
; legalization uses the correct vector size limits.

; CHECK-DAG: OpCapability Shader
; CHECK-DAG: OpEntryPoint GLCompute %[[#entry:]] "main"
Comment thread
MrSidims marked this conversation as resolved.

define <4 x i32> @helper(<4 x i32> %a, <4 x i32> %b) {
entry:
%result = add <4 x i32> %a, %b
ret <4 x i32> %result
}

define void @main() #0 {
entry:
%a = call <4 x i32> @helper(<4 x i32> <i32 1, i32 2, i32 3, i32 4>, <4 x i32> <i32 5, i32 6, i32 7, i32 8>)
ret void
}

attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
Loading