[PATCH] D132942: [PowerPC][GISel]add support for float point arithmetic operations

[Kai Nacke via Phabricator via llvm-commits]

Kai added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/GISel/PPCInstructionSelector.cpp:93
+
 static bool selectCopy(MachineInstr &I, const TargetInstrInfo &TII,
                        MachineRegisterInfo &MRI, const TargetRegisterInfo &TRI,
----------------
nemanjai wrote:
> It seems that this function does something sensible in 3 of 4 possible situations:
> 
> 
>   # Both SrcReg and DstReg are physical regs, just return `true`
>   # DstReg is virtual and SrcReg is physical, constrain DstReg
>   # DstReg is physical and SrcReg is virtual, constrain SrcReg
>   # Both DstReg and SrcReg are virtual, constrain DstReg and leave SrcReg untouched
> 
> Why do we behave this way for 4. above?
> 
Good point. I believe the outlier is case 3. The copy instruction is somewhat special. My understanding is that we need to care only about the DstReg, because the SrcReg is either a physical register or is constraint somewhere else. Since most of the code comes from my PR, my proposal is that I change the code in my PR to


```
  for (const MachineOperand &MO : I.operands()) {
    if (MO.getReg().isPhysical())
      continue;

    const TargetRegisterClass *RC =
            TRI.getConstrainedRegClassForOperand(MO, *MRI);
    if (!RC)
      continue;
    RBI.constrainGenericRegister(MO.getReg(), *RC, *MRI);
  }
  return true;
```

This is imho more uniform, and better to read. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132942