Jonas Paulsson via llvm-dev
2018-May-30 13:02 UTC
[llvm-dev] InstrEmitter::CreateVirtualRegisters handling of CopyToReg
Hi, I wonder if anyone has any comment on a patch like: diff --git a/lib/CodeGen/SelectionDAG/InstrEmitter.cpp b/lib/CodeGen/SelectionDAG/InstrEmitter.cpp index 65ee3816f84..4780f6f0e59 100644 --- a/lib/CodeGen/SelectionDAG/InstrEmitter.cpp +++ b/lib/CodeGen/SelectionDAG/InstrEmitter.cpp @@ -243,18 +243,21 @@ void InstrEmitter::CreateVirtualRegisters(SDNode *Node, if (!VRBase && !IsClone && !IsCloned) for (SDNode *User : Node->uses()) { if (User->getOpcode() == ISD::CopyToReg && User->getOperand(2).getNode() == Node && User->getOperand(2).getResNo() == i) { unsigned Reg = cast<RegisterSDNode>(User->getOperand(1))->getReg(); if (TargetRegisterInfo::isVirtualRegister(Reg)) { - const TargetRegisterClass *RegRC = MRI->getRegClass(Reg); - if (RegRC == RC) { + // Allow constraining the virtual register's class within reason, + // just like what AddRegisterOperand will allow. + const TargetRegisterClass *ConstrainedRC + = MRI->constrainRegClass(Reg, RC, MinRCSize); + if (ConstrainedRC) { VRBase = Reg; MIB.addReg(VRBase, RegState::Define); break; } } } } Why do the register classes currently have to match exactly in this case? It seems that these COPYs that now remain may end up in the same register class, if the users require it. So why not constrain also here directly, if this is done generally when the register is used as input? /Jonas
Jonas Paulsson via llvm-dev
2018-Jun-09 06:37 UTC
[llvm-dev] Fwd: InstrEmitter::CreateVirtualRegisters handling of CopyToReg
Hi, It would still be nice to get some comment about this patch which seems to help on SystemZ in the case where GRX32 pseudo instructions are used. This means that the register eventually ends up either in GR32 or GRH32 (low or high 32 bit subregs). Basically, I wonder if anyone has tried this before and has any argument against this? Thanks, Jonas -------- Forwarded Message -------- Subject: InstrEmitter::CreateVirtualRegisters handling of CopyToReg Date: Wed, 30 May 2018 15:02:57 +0200 From: Jonas Paulsson <paulsson at linux.vnet.ibm.com> To: llvm-dev <llvm-dev at lists.llvm.org> CC: Ulrich Weigand <ulrich.weigand at de.ibm.com> Hi, I wonder if anyone has any comment on a patch like: diff --git a/lib/CodeGen/SelectionDAG/InstrEmitter.cpp b/lib/CodeGen/SelectionDAG/InstrEmitter.cpp index 65ee3816f84..4780f6f0e59 100644 --- a/lib/CodeGen/SelectionDAG/InstrEmitter.cpp +++ b/lib/CodeGen/SelectionDAG/InstrEmitter.cpp @@ -243,18 +243,21 @@ void InstrEmitter::CreateVirtualRegisters(SDNode *Node, if (!VRBase && !IsClone && !IsCloned) for (SDNode *User : Node->uses()) { if (User->getOpcode() == ISD::CopyToReg && User->getOperand(2).getNode() == Node && User->getOperand(2).getResNo() == i) { unsigned Reg cast<RegisterSDNode>(User->getOperand(1))->getReg(); if (TargetRegisterInfo::isVirtualRegister(Reg)) { - const TargetRegisterClass *RegRC = MRI->getRegClass(Reg); - if (RegRC == RC) { + // Allow constraining the virtual register's class within reason, + // just like what AddRegisterOperand will allow. + const TargetRegisterClass *ConstrainedRC + = MRI->constrainRegClass(Reg, RC, MinRCSize); + if (ConstrainedRC) { VRBase = Reg; MIB.addReg(VRBase, RegState::Define); break; } } } } Why do the register classes currently have to match exactly in this case? It seems that these COPYs that now remain may end up in the same register class, if the users require it. So why not constrain also here directly, if this is done generally when the register is used as input? /Jonas -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180609/b6dd431b/attachment-0001.html>
Friedman, Eli via llvm-dev
2018-Jun-11 22:00 UTC
[llvm-dev] Fwd: InstrEmitter::CreateVirtualRegisters handling of CopyToReg
I haven't had the occasion to touch this particular part of InstrEmitter, but the patch looks reasonable. -Eli On 6/8/2018 11:37 PM, Jonas Paulsson wrote:> > Hi, > > It would still be nice to get some comment about this patch which > seems to help on SystemZ in the case where GRX32 pseudo instructions > are used. This means that the register eventually ends up either in > GR32 or GRH32 (low or high 32 bit subregs). > > Basically, I wonder if anyone has tried this before and has any > argument against this? > > Thanks, > > Jonas > > > -------- Forwarded Message -------- > Subject: InstrEmitter::CreateVirtualRegisters handling of CopyToReg > Date: Wed, 30 May 2018 15:02:57 +0200 > From: Jonas Paulsson <paulsson at linux.vnet.ibm.com> > To: llvm-dev <llvm-dev at lists.llvm.org> > CC: Ulrich Weigand <ulrich.weigand at de.ibm.com> > > > > Hi, > > I wonder if anyone has any comment on a patch like: > > diff --git a/lib/CodeGen/SelectionDAG/InstrEmitter.cpp > b/lib/CodeGen/SelectionDAG/InstrEmitter.cpp > index 65ee3816f84..4780f6f0e59 100644 > --- a/lib/CodeGen/SelectionDAG/InstrEmitter.cpp > +++ b/lib/CodeGen/SelectionDAG/InstrEmitter.cpp > @@ -243,18 +243,21 @@ void InstrEmitter::CreateVirtualRegisters(SDNode > *Node, > > if (!VRBase && !IsClone && !IsCloned) > for (SDNode *User : Node->uses()) { > if (User->getOpcode() == ISD::CopyToReg && > User->getOperand(2).getNode() == Node && > User->getOperand(2).getResNo() == i) { > unsigned Reg > cast<RegisterSDNode>(User->getOperand(1))->getReg(); > if (TargetRegisterInfo::isVirtualRegister(Reg)) { > - const TargetRegisterClass *RegRC = MRI->getRegClass(Reg); > - if (RegRC == RC) { > + // Allow constraining the virtual register's class within > reason, > + // just like what AddRegisterOperand will allow. > + const TargetRegisterClass *ConstrainedRC > + = MRI->constrainRegClass(Reg, RC, MinRCSize); > + if (ConstrainedRC) { > VRBase = Reg; > MIB.addReg(VRBase, RegState::Define); > break; > } > } > } > } > > Why do the register classes currently have to match exactly in this case? > > It seems that these COPYs that now remain may end up in the same > register class, if the users require it. So why not constrain also here > directly, if this is done generally when the register is used as input? > > /Jonas > >-- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180611/b67d1109/attachment.html>
Reasonably Related Threads
- [LLVMdev] TableGen Register Class not matching for MI in 3.6
- [LLVMdev] TableGen Register Class not matching for MI in 3.6
- [LLVMdev] TableGen Register Class not matching for MI in 3.6
- TableGen register class
- [LLVMdev] What does this error mean: psuedo instructions should be removed before code emission?