[PATCH] D116465: [SPIRV 6/6] Add the module analysis pass and the simplest tests

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp:49
+      : AsmPrinter(TM, std::move(Streamer)), STI(TM.getMCSubtargetInfo()) {
+    ST = static_cast<const SPIRVTargetMachine &>(TM).getSubtargetImpl();
+    TII = ST->getInstrInfo();
----------------
iliya-diyachkov wrote:
> arsenm wrote:
> > iliya-diyachkov wrote:
> > > arsenm wrote:
> > > > getSubtarget<SPIRVSubtarget>() instead of static_cast
> > > As I see we need to have either MachineFunction or Function to call getSubtarget<SPIRVSubtarget>().
> > > However in SPIRVAsmPrinter ST is required even if there are no functions in the module. I don't see any other way than to use static_cast as some other targets do (e.g. in NVPTXAsmPrinter::emitStartOfAsmFile or in AVRAsmPrinter::emitStartOfAsmFile). If you know, please advise.
> > Oh, this points out a deeper issue that you're using the global subtarget. Ideally you're always querying the subtarget from the context function
> I understand this, but in some cases we have a module without functions, and we still need to use Subtarget.
> 
> We could get Subtarget using static_cast when initializing AsmPrinter.  Then if there are MachineFunctions in the module, we will get Subtarget using MF.getSubtarget<SPIRVSubtarget>() in emitFunctionHeader() for each MachineFunction.
> Does this approach make sense?
Yes, each function should query the subtarget


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

https://reviews.llvm.org/D116465