[llvm-commits] Fixing Bug 13662: paired register for inline asm with 64-bit data on ARM

[Weiming Zhao]

The change to InstrEmitter is to let it support ExtractSubReg / RegSequence
to be connected to RegisterNode and  CopyFrom node to be connected to a
SubReg SDNode.

 

Normally, ExtractSubReg/RegSequence are only connected to a SDValue, but for
inlineasem, we have to connect them to a VR.

Existing inlineasm just emits multiple 32bit-typed VRs, it doesn’t need
ExtractSubReg/RegSequence, so such scenarios are not encountered. 

 

Now, this issue is exposed . 

So my patch let it handle more cases. It should be safe. For example:

 

@@ -478,8 +478,12 @@ void InstrEmitter::EmitSubregNode(SDNode *Node,

     // classes.

     unsigned SubIdx =
cast<ConstantSDNode>(Node->getOperand(1))->getZExtValue();

     const TargetRegisterClass *TRC =
TLI->getRegClassFor(Node->getValueType(0));

+    unsigned VReg;

+    if (RegisterSDNode *R = dyn_cast<RegisterSDNode>(Node->getOperand(0)))
è let SubReg support a RegisterSDNode

+      VReg = R->getReg();

+    else

+      VReg = getVR(Node->getOperand(0), VRBaseMap); è Original path

-    unsigned VReg = getVR(Node->getOperand(0), VRBaseMap);

     MachineInstr *DefMI = MRI->getVRegDef(VReg);

     unsigned SrcReg, DstReg, DefSubIdx;

     if (DefMI &&

@@ -610,7 +614,8 @@ void InstrEmitter::EmitRegSequence(SDNode *Node,

       // insert copies for them in TwoAddressInstructionPass anyway.

       if (!R || !TargetRegisterInfo::isPhysicalRegister(R->getReg())) {

         unsigned SubIdx = cast<ConstantSDNode>(Op)->getZExtValue();

-        unsigned SubReg = getVR(Node->getOperand(i-1), VRBaseMap);

+        unsigned SubReg = R ? R->getReg() :

+          getVR(Node->getOperand(i-1), VRBaseMap);  è let RegSequence
support a RegisterSDNode

         const TargetRegisterClass *TRC = MRI->getRegClass(SubReg);

         const TargetRegisterClass *SRC =

         TRI->getMatchingSuperRegClass(RC, TRC, SubIdx);

@@ -861,7 +866,12 @@ EmitSpecialNode(SDNode *Node, bool IsClone, bool
IsCloned,

     break;

   }

   case ISD::CopyFromReg: {

-    unsigned SrcReg = cast<RegisterSDNode>(Node->getOperand(1))->getReg();

+    unsigned SrcReg;

+    if (Node->getOperand(1).isMachineOpcode() &&

+        Node->getOperand(1).getMachineOpcode() ==
TargetOpcode::EXTRACT_SUBREG) è support copyFrom a Extract_SubReg

+      SrcReg = getVR(Node->getOperand(1), VRBaseMap);

+    else

+      SrcReg = cast<RegisterSDNode>(Node->getOperand(1))->getReg(); è
original path

     EmitCopyFromReg(Node, 0, IsClone, IsCloned, SrcReg, VRBaseMap);

     break;

 

 

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
The Linux Foundation

 

From: Jakob Stoklund Olesen [mailto:stoklund at 2pi.dk] 
Sent: Monday, December 17, 2012 9:43 AM
To: weimingz at codeaurora.org
Cc: 'Commit Messages and Patches for LLVM'; 'James Grosbach'; 'Evan Cheng'
Subject: Re: [llvm-commits] Fixing Bug 13662: paired register for inline asm
with 64-bit data on ARM

 

 

On Dec 12, 2012, at 10:55 AM, Weiming Zhao <weimingz at codeaurora.org> wrote:





The difference between dealing with linlineasm and ldrexd/strexd intrisics
is that: for intrinsics, we lower them to MachineNode, thus the framework
can get the corresponding register class via td file. For example, for
ldrexd, we only need to put a ValueType of MVT::Untyped and the framework
will allocate GPRPairRegClass.

 

Right.





However, for inlineasm, it doesn’t check the instructions inside of it. So
we have to manually create VRs for it and copy results back.

 

Yes. That is how all inline asm arguments work already AFAICT.

 

I’m attaching my original patch.

 

The changes you are proposing to InstrEmitter.cpp are not safe, and since
inline asm already uses virtual registers, I don't understand why they
should be necessary.

 

 

/jakob

 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121219/b1d23297/attachment.html>