[PATCH] D116464: [SPIRV 5/6] Add LegalizerInfo, InstructionSelector and utilities

[Aleksandr Bezzubikov via Phabricator via llvm-commits]

zuban32 added inline comments.


================
Comment at: llvm/lib/Target/SPIRV/SPIRVLegalizerInfo.cpp:361-365
+  MachineInstr &AssignTypeInst =
+      *(MRI.use_instr_begin(MI.getOperand(0).getReg()));
+  auto NewReg = createNewIdReg(MI.getOperand(0).getReg(), Opc, MRI, *GR).first;
+  AssignTypeInst.getOperand(1).setReg(NewReg);
+  MI.getOperand(0).setReg(NewReg);
----------------
zuban32 wrote:
> arsenm wrote:
> > zuban32 wrote:
> > > iliya-diyachkov wrote:
> > > > arsenm wrote:
> > > > > You're assuming there's only a single use, and your legalize action should only touch the passed instruction
> > > > Instructions processed here should always have a single use which is a pseudo instruction ASSIGN_TYPE, generated by SPIRV specific pass before the legalizer. We'll add a check that this condition is met. Also it looks like the modification of another instructions may be avoided. 
> > > > And this fragment works together with another pass that is not presented in this series, so I would postpone it for now. 
> > > The single-use constraint is enforced by the design of our target, every non-void instruction have the only user - type assignment instruction, which in turn may have multiple users just as the original instruction before.
> > This is not a property the target can decide, the incoming IR can have multiple uses. It's simply wrong to rely on it like this
> Why not if we have a target-specific LLVM IR pass making that assumption always true?
Oh, I see. It's just the pass responsible for making the input IR compliant with what SPIR-V specific GlobalISel stages expect is currently missing from these patches. Maybe we need to extend the series


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116464