[SPIRV] Add support for SPV_INTEL_optnone

[pmatos]

Summary

Adds support for SPV_INTEL_optnone.

[pmatos]

This is a draft patch - I want to discuss somethings inline.

The output for optnone.ll is:

bin/llc -O0 -mtriple=spirv32-unknown-unknown ../llvm/test/CodeGen/SPIRV/optnone.ll -o -
	OpCapability Kernel
	OpCapability Addresses
	OpCapability Linkage
	%1 = OpExtInstImport "OpenCL.std"
	OpMemoryModel Physical32 OpenCL
	OpExecutionMode %4 ContractionOff
	OpSource Unknown 0
	OpName %4 "_Z3foov"
	OpDecorate %4 LinkageAttributes "_Z3foov" Export
	%2 = OpTypeVoid
	%3 = OpTypeFunction %2
	OpCapability OptNoneINTEL               ; -- Begin function _Z3foov
	OpExtension "�" "�" "�" "�" "�" "�"
	%4 = OpFunction %2 DontInline %3
	%5 = OpLabel
	OpReturn
	OpFunctionEnd

OpExtension emits some funky unicode characters, I feel like I am doing this incorrectly but similarly to what's being done in SPIRVAsmPrinter::outputGlobalRequirements() :

MCInst Inst;
    Inst.setOpcode(SPIRV::OpExtension);
    addStringImm(getSymbolicOperandMnemonic(
                     SPIRV::OperandCategory::ExtensionOperand, Ext),
                 Inst);
    outputMCInst(Inst);

The other issue is that afaiu the OpExtension declaractions need to come up the top of the module. How is this organized in the backend so that I can emit these at the top of the module? Do I need to add these to the module requirements somehow?

llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp

127 ↗

(On Diff #544243)

Is this the best place to issue the instruction or is it preferable to do so during lowering for each function?

[pmatos]

I just checked section 2.4 of the spirv spec and all caps and exts need to come at the beginning: https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_logical_layout_of_a_module

So actually, I will move this code to a pass on MFs that adds the required extension and caps to the module.

[mpaszkowski]

In D156297#4534419, @pmatos wrote:

This is a draft patch - I want to discuss somethings inline.

The output for optnone.ll is:

bin/llc -O0 -mtriple=spirv32-unknown-unknown ../llvm/test/CodeGen/SPIRV/optnone.ll -o -
	OpCapability Kernel
	OpCapability Addresses
	OpCapability Linkage
	%1 = OpExtInstImport "OpenCL.std"
	OpMemoryModel Physical32 OpenCL
	OpExecutionMode %4 ContractionOff
	OpSource Unknown 0
	OpName %4 "_Z3foov"
	OpDecorate %4 LinkageAttributes "_Z3foov" Export
	%2 = OpTypeVoid
	%3 = OpTypeFunction %2
	OpCapability OptNoneINTEL               ; -- Begin function _Z3foov
	OpExtension "�" "�" "�" "�" "�" "�"
	%4 = OpFunction %2 DontInline %3
	%5 = OpLabel
	OpReturn
	OpFunctionEnd

OpExtension emits some funky unicode characters, I feel like I am doing this incorrectly but similarly to what's being done in SPIRVAsmPrinter::outputGlobalRequirements() :

MCInst Inst;
    Inst.setOpcode(SPIRV::OpExtension);
    addStringImm(getSymbolicOperandMnemonic(
                     SPIRV::OperandCategory::ExtensionOperand, Ext),
                 Inst);
    outputMCInst(Inst);

The other issue is that afaiu the OpExtension declaractions need to come up the top of the module. How is this organized in the backend so that I can emit these at the top of the module? Do I need to add these to the module requirements somehow?

Thank you for the patch @pmatos!

The OpExtension instructions should be added earlier in the pipeline. If the OpExtension instruction is added explicitly to MIR, it should be added before SPIRVModuleAnalysis. All explicit OpExtension instructions are then collected in this method. Please take a look at the draft pull request adding an extension here -- this should also help address the issue with incorrect unicode characters printed out in the SPIR-V assembly.

There are draft patches splitting SPIRVModuleAnalysis into smaller passes on our GitHub, but this can be done later (I am currently a bit busy, but promise to get back to this in August).

[iliya-diyachkov]

Paulo, please also add a lit test which checks the new feature.

llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp

127 ↗

(On Diff #544243)

A more appropriate place to add the required capability is inside collectReqs() in SPIRVModuleAnalysis.cpp.

As for the extension, I'm not sure we should add it by default, having a function with optnone. I assume that SPIRV translator issues OpExtension instructions if we pass a command line option (like this --spirv-ext=+SPV_INTEL_optnone) and have a corresponding trigger in IR (like a function with optnone). Probably other ways to enable desirable extensions are suitable (e.g. in module metadata).

We don't have support for the command line option yet, perhaps it can be implemented inside SPIRVSubtarget::initAvailableExtensions using cl::opt<string> and string parsing. Then we check function's optnone and availability of the extension inside collectReqs() and add the extension, as we do with capabilities.

[pmatos]

llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp

127 ↗

(On Diff #544243)

Thanks Iliya, I think for now I will add it by default if there's a function with optnone. I think later once we get some command line arguments, we can make it optional depending on if the command line argument is present or not. I think Michal already has some code in this direction so I would prefer no to duplicate this.

[pmatos]

Add INTEL optnone work to PreLegalizer.
Alternatively this could be done on its own pass. Doing it here runs the risk
of just using the PreLegalizer to too much of this kind of work.

In any case we are now generating better code with extension
declaration and capability at the top of the function.

[pmatos]

Current patch generates:

	OpCapability Kernel
	OpCapability Addresses
	OpCapability Linkage
	OpCapability OptNoneINTEL
	OpExtension "SPV_INTEL_optnone"
	%1 = OpExtInstImport "OpenCL.std"
	OpMemoryModel Physical32 OpenCL
	OpExecutionMode %4 ContractionOff
	OpSource Unknown 0
	OpName %4 "_Z3foov"
	OpDecorate %4 LinkageAttributes "_Z3foov" Export
	%2 = OpTypeVoid
	%3 = OpTypeFunction %2
	%4 = OpFunction %2 DontInline %3        ; -- Begin function _Z3foov
	%5 = OpLabel
	OpReturn
	OpFunctionEnd
                                        ; -- End function

which looks good to me. If you think this approach works well, I will tidy up the patch and fix up the test.

[pmatos]

Update test.

[pmatos]

Remove unnecessary header include.

[pmatos]

This is ready for review. Will wait for further comments.

[iliya-diyachkov]

In D156297#4545883, @pmatos wrote:

Add INTEL optnone work to PreLegalizer.
Alternatively this could be done on its own pass. Doing it here runs the risk
of just using the PreLegalizer to too much of this kind of work.

In any case we are now generating better code with extension
declaration and capability at the top of the function.

The output code is Ok and these instructions can be generated at this place. However, I think it's better to make such instructions as late as possible. If we insert this code into collectReqs (SPIRVModuleAnalysis.cpp), the output will be the same:

@@ -898,6 +902,12 @@ static void collectReqs(const Module &M, SPIRV::ModuleAnalysisInfo &MAI,
       MAI.Reqs.getAndAddRequirements(
           SPIRV::OperandCategory::ExecutionModeOperand,
           SPIRV::ExecutionMode::VecTypeHint, ST);
+    if (F.hasOptNone() &&
+        ST.canUseExtension(SPIRV::Extension::SPV_INTEL_optnone)) {
+      // Output OpCapability OptNoneINTEL.
+      MAI.Reqs.addExtension(SPIRV::Extension::SPV_INTEL_optnone);
+      MAI.Reqs.addCapability(SPIRV::Capability::OptNoneINTEL);
+    }
   }
 }

but we even don't generate the instructions in IR (since we add exts and caps directly to requirements), so IR becomes simpler and there is less work for the backend.

llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp

587–596 ↗

(On Diff #545553)

Please format this code properly (i.e. you can apply clang-format to all c++ files).

llvm/test/CodeGen/SPIRV/optnone.ll

18

Don't need this empty line?

19–22

Is it necessary to move this check after the foo() declaration? Maybe it's better to leave it in the same place and put OpCapability/OpExtension checks before it.

[iliya-diyachkov]

llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp

555

Please add '.' at the end of the sentence.

[pmatos]

Address Iliya's comments. Thanks for the review.

[pmatos]

Run clang-format.

[iliya-diyachkov]

Thanks, Paulo. The patch looks good to me.

llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp

void RequirementHandler::initAvailableCapabilities(const SPIRVSubtarget &ST) {
   if (ST.isAtLeastSPIRVVer(14))
     addAvailableCaps({Capability::DenormPreserve, Capability::DenormFlushToZero,
                       Capability::SignedZeroInfNanPreserve,
                       Capability::RoundingModeRTE,
                       Capability::RoundingModeRTZ});
   // TODO: verify if this needs some checks.
   addAvailableCaps({Capability::Float16, Capability::Float64});
 
+  // Add cap for SPV_INTEL_optnone.
+  // FIXME: this should be added only if the target has the extension.
+  addAvailableCaps({Capability::OptNoneINTEL});
+
   // TODO: add OpenCL extensions.
 }
 } // namespace SPIRV
 } // namespace llvm
 
 // Add the required capabilities from a decoration instruction (including
 // BuiltIns).
 static void addOpDecorateReqs(const MachineInstr &MI, unsigned DecIndex,
for (auto FI = M.begin(), E = M.end(); FI != E; ++FI) {
     if (F.getMetadata("intel_reqd_sub_group_size"))
       MAI.Reqs.getAndAddRequirements(
           SPIRV::OperandCategory::ExecutionModeOperand,
           SPIRV::ExecutionMode::SubgroupSize, ST);
     if (F.getMetadata("vec_type_hint"))
       MAI.Reqs.getAndAddRequirements(
           SPIRV::OperandCategory::ExecutionModeOperand,
           SPIRV::ExecutionMode::VecTypeHint, ST);
+
+    if (F.hasOptNone() &&
+        ST.canUseExtension(SPIRV::Extension::SPV_INTEL_optnone)) {
+      // Output OpCapability OptNoneINTEL.
+      MAI.Reqs.addExtension(SPIRV::Extension::SPV_INTEL_optnone);
+      MAI.Reqs.addCapability(SPIRV::Capability::OptNoneINTEL);
+    }
   }
 }
 
 static unsigned getFastMathFlags(const MachineInstr &I) {
   unsigned Flags = SPIRV::FPFastMathMode::None;
   if (I.getFlag(MachineInstr::MIFlag::FmNoNans))
     Flags |= SPIRV::FPFastMathMode::NotNaN;
   if (I.getFlag(MachineInstr::MIFlag::FmNoInfs))

llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp

 }
 
 // TODO: use command line args for this rather than defaults.
 void SPIRVSubtarget::initAvailableExtensions() {
   AvailableExtensions.clear();
   if (!isOpenCLEnv())
     return;
   // A default extension for testing.
+  // FIXME: This should be changed when we can select extensions through a
+  // command line flag.
   AvailableExtensions.insert(
       SPIRV::Extension::SPV_KHR_no_integer_wrap_decoration);
+  AvailableExtensions.insert(SPIRV::Extension::SPV_INTEL_optnone);
 }
 
 // TODO: use command line args for this rather than just defaults.
 // Must have called initAvailableExtensions first.
 void SPIRVSubtarget::initAvailableExtInstSets() {
   AvailableExtInstSets.clear();
   if (!isOpenCLEnv())
     AvailableExtInstSets.insert(SPIRV::InstructionSet::GLSL_std_450);

llvm/lib/Target/SPIRV/SPIRVSymbolicOperands.td

 defm SPV_INTEL_shader_integer_functions2 : ExtensionOperand<50>;
 defm SPV_INTEL_fpga_loop_controls : ExtensionOperand<51>;
 defm SPV_EXT_fragment_shader_interlock : ExtensionOperand<52>;
 defm SPV_NV_shader_sm_builtins : ExtensionOperand<53>;
 defm SPV_KHR_shader_clock : ExtensionOperand<54>;
 defm SPV_INTEL_unstructured_loop_controls : ExtensionOperand<55>;
 defm SPV_EXT_demote_to_helper_invocation : ExtensionOperand<56>;
 defm SPV_INTEL_fpga_reg : ExtensionOperand<57>;
+defm SPV_INTEL_optnone : ExtensionOperand<58>;
 
 //===----------------------------------------------------------------------===//
 // Multiclass used to define Capabilities enum values and at the same time
 // SymbolicOperand entries with string mnemonics, versioning, extensions, and
 // capabilities.
 //===----------------------------------------------------------------------===//
 
 def Capability : GenericEnum, Operand<i32> {

 defm VulkanMemoryModelDeviceScopeKHR : CapabilityOperand<5346, 0, 0, [], []>;
 defm ImageFootprintNV : CapabilityOperand<5282, 0, 0, [], []>;
 defm FragmentBarycentricNV : CapabilityOperand<5284, 0, 0, [], []>;
 defm ComputeDerivativeGroupQuadsNV : CapabilityOperand<5288, 0, 0, [], []>;
 defm ComputeDerivativeGroupLinearNV : CapabilityOperand<5350, 0, 0, [], []>;
 defm FragmentDensityEXT : CapabilityOperand<5291, 0, 0, [], [Shader]>;
 defm PhysicalStorageBufferAddressesEXT : CapabilityOperand<5347, 0, 0, [], [Shader]>;
 defm CooperativeMatrixNV : CapabilityOperand<5357, 0, 0, [], [Shader]>;
+defm OptNoneINTEL : CapabilityOperand<6094, 0, 0, [SPV_INTEL_optnone], []>;
 
 //===----------------------------------------------------------------------===//
 // Multiclass used to define SourceLanguage enum values and at the same time
 // SymbolicOperand entries.
 //===----------------------------------------------------------------------===//
 
 def SourceLanguage : GenericEnum, Operand<i32> {
   let FilterClass = "SourceLanguage";

llvm/test/CodeGen/SPIRV/optnone.ll

 ;; Check that optnone is correctly ignored when extension is not enabled
 ; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s --check-prefix=CHECK-SPIRV
 
+; CHECK-SPIRV: OpCapability OptNoneINTEL
+; CHECK-SPIRV: OpExtension "SPV_INTEL_optnone"
+
 ;; Per SPIR-V spec:
 ;; FunctionControlDontInlineMask = 0x2 (2)
 ; CHECK-SPIRV: %[[#]] = OpFunction %[[#]] DontInline
 
 ; Function Attrs: nounwind optnone noinline
 define spir_func void @_Z3foov() #0 {
 entry:
   ret void
 }
 
 attributes #0 = { nounwind optnone noinline }