[PATCH] D127530: [PowerPC] Extend GlobalISel implementation to emit and/or/xor.

[Kai Nacke via Phabricator via llvm-commits]

Kai marked an inline comment as done.
Kai added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/GISel/PPCInstructionSelector.cpp:94
+  Register DstReg = I.getOperand(0).getReg();
+  if (Register::isPhysicalRegister(DstReg))
+    return true;
----------------
arsenm wrote:
> Kai wrote:
> > arsenm wrote:
> > > You still need to at least constrain the source register if it's virtual. You probably should have some MIR tests for every permutation of physical / virtual register copies
> > Is this actually needed? I added the code but I see no change.
> > If I compare this with mipsel-linux:
> > 
> > Before `instruction-selection`:
> > 
> > ```
> >   %2:gprb(s32) = COPY $a0
> > ...
> >   $v0 = COPY %13(s32)
> > ```
> > 
> > and after `instruction-selection`:
> > 
> > ```
> >   %2:gpr32 = COPY $a0
> > ...
> >   $v0 = COPY %13
> > ```
> > 
> > So during instruction selection, for the destination virtual register, the register bank is replaced with the actual register class. But in the other case, the virtual source register has no constraint after the `instruction-select`.
> > I get similar output when also constraining the source virtual register.  
> Most of the time it works, since the use constrain also happens to work for the dest instruction. However there are some cases where this doesn't happen, leaving an unconstrained register. You'll need to have different kinds of uses and defs. I can't think of a nice example at the moment
Thanks, I see. Changed the code to constrain all registers.


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

https://reviews.llvm.org/D127530