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

Mon Jul 31 00:41:16 PDT 2023

pmatos added inline comments.

================
Comment at: llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp:127
+    Inst.setOpcode(SPIRV::OpCapability);
+    Inst.addOperand(MCOperand::createImm(static_cast<unsigned>(
+        SPIRV::Capability::OptNoneINTEL)));
----------------
iliya-diyachkov wrote:
> pmatos wrote:
> > Is this the best place to issue the instruction or is it preferable to do so during lowering for each function?
> 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.
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. 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156297