[PATCH] D144897: [SPIRV] fix several issues in builds with expensive checks

[Ilia Diachkov via Phabricator via llvm-commits]

iliya-diyachkov marked an inline comment as done.
iliya-diyachkov added inline comments.


================
Comment at: llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp:392
       assert(Arg.Regs.size() == 1 && "Call arg has multiple VRegs");
+      if (!MRI->getRegClassOrNull(Arg.Regs[0]))
+        MRI->setRegClass(Arg.Regs[0], &SPIRV::IDRegClass);
----------------
arsenm wrote:
> zuban32 wrote:
> > iliya-diyachkov wrote:
> > > arsenm wrote:
> > > > I'd expect the class to never be set at this point (if not, I'd expect it to be always set)
> > > 18 LIT tests will fail if I remove this setRegClass(). On other hand, I found calls with register classes already set here in 27 LIT tests.
> > > So I suppose it makes sense to leave this check and setting.
> > @arsenm could you pls elaborate? Is it supposed to be set later (if so, where)? I thought CallLowering being a part of IRTranslator is a perfect place to set reg classes as vregs have just been created
> If the instruction had a register constraint it should have been set to begin with. You are fixing up invalid mir after the fact 
I removed this fixing and added the required `setRegClass`s in SPIRVBuiltins.cpp. Now we set reg classes only when replacing calls with machine instructions.


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

https://reviews.llvm.org/D144897