[PATCH] D156297: [SPIRV] Add support for SPV_INTEL_optnone

[Ilia Diachkov via Phabricator via llvm-commits]

iliya-diyachkov added a comment.

In D156297#4545883 <https://reviews.llvm.org/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.



================
Comment at: llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp:587-596
+   MachineIRBuilder MIRBuilder(EntryBB, EntryBB.begin());
+
+  if (F.hasOptNone() && ST.canUseExtension(
+          SPIRV::Extension::SPV_INTEL_optnone)) {
+    // Output OpCapability OptNoneINTEL.
+    MIRBuilder.buildInstr(SPIRV::OpExtension)
+    .addImm(SPIRV::Extension::SPV_INTEL_optnone);
----------------
Please format this code properly (i.e. you can apply clang-format to all c++ files).


================
Comment at: llvm/test/CodeGen/SPIRV/optnone.ll:12-15
+
+;; Per SPIR-V spec:
+;; FunctionControlDontInlineMask = 0x2 (2)
+; CHECK-SPIRV: %[[#]] = OpFunction %[[#]] DontInline
----------------
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.


================
Comment at: llvm/test/CodeGen/SPIRV/optnone.ll:17
 attributes #0 = { nounwind optnone noinline }
+
----------------
Don't need this empty line?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156297/new/

https://reviews.llvm.org/D156297