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

[Ilia Diachkov via Phabricator via llvm-commits]

iliya-diyachkov 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();
----------------
arsenm wrote:
> 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
Ok, I'll implement it later today.


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

https://reviews.llvm.org/D116465