[llvm] r175088 - Bug fix 13622: Add paired register support for inline asm with 64-bit data on ARM
Bill Wendling
wendling at apple.com
Wed Feb 13 15:22:12 PST 2013
Please revert it in the meantime. Thanks.
-bw
On Feb 13, 2013, at 3:19 PM, Weiming Zhao <weimingz at codeaurora.org> wrote:
> Hi,
>
> I'm checking that. Sorry for the inconvenience.
>
> Weiming
>
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
> The Linux Foundation
>
>
> -----Original Message-----
> From: NAKAMURA Takumi [mailto:geek4civic at gmail.com]
> Sent: Wednesday, February 13, 2013 3:11 PM
> To: Weiming Zhao
> Cc: llvm-commits
> Subject: Re: [llvm] r175088 - Bug fix 13622: Add paired register support for
> inline asm with 64-bit data on ARM
>
> It causes an assertion failure...
>
> Assertion failed: isReg() && "This is not a register operand!", file
> llvm/include/llvm/CodeGen/MachineOperand.h, line 260
> http://bb.pgr.jp/builders/cmake-clang-i686-mingw32/builds/3121
>
> 2013/2/14 Weiming Zhao <weimingz at codeaurora.org>:
>> Author: weimingz
>> Date: Wed Feb 13 15:43:02 2013
>> New Revision: 175088
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=175088&view=rev
>> Log:
>> Bug fix 13622: Add paired register support for inline asm with 64-bit
>> data on ARM
>>
>> Added:
>> llvm/trunk/test/CodeGen/ARM/inlineasm-64bit.ll
>> Modified:
>> llvm/trunk/lib/Target/ARM/ARMAsmPrinter.cpp
>> llvm/trunk/lib/Target/ARM/ARMISelDAGToDAG.cpp
>> llvm/trunk/test/CodeGen/ARM/arm-modifier.ll
>>
>> Modified: llvm/trunk/lib/Target/ARM/ARMAsmPrinter.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMAsmPr
>> inter.cpp?rev=175088&r1=175087&r2=175088&view=diff
>> ======================================================================
>> ========
>> --- llvm/trunk/lib/Target/ARM/ARMAsmPrinter.cpp (original)
>> +++ llvm/trunk/lib/Target/ARM/ARMAsmPrinter.cpp Wed Feb 13 15:43:02
>> +++ 2013
>> @@ -342,6 +342,11 @@ void ARMAsmPrinter::printOperand(const M
>> unsigned Reg = MO.getReg();
>> assert(TargetRegisterInfo::isPhysicalRegister(Reg));
>> assert(!MO.getSubReg() && "Subregs should be eliminated!");
>> + if(ARM::GPRPairRegClass.contains(Reg)) {
>> + const MachineFunction &MF = *MI->getParent()->getParent();
>> + const TargetRegisterInfo *TRI = MF.getTarget().getRegisterInfo();
>> + Reg = TRI->getSubReg(Reg, ARM::gsub_0);
>> + }
>> O << ARMInstPrinter::getRegisterName(Reg);
>> break;
>> }
>> @@ -530,14 +535,12 @@ bool ARMAsmPrinter::PrintAsmOperand(cons
>> const MachineOperand &MO = MI->getOperand(OpNum);
>> if (!MO.isReg())
>> return true;
>> - const TargetRegisterClass &RC = ARM::GPRRegClass;
>> const MachineFunction &MF = *MI->getParent()->getParent();
>> const TargetRegisterInfo *TRI =
>> MF.getTarget().getRegisterInfo();
>> -
>> - unsigned RegIdx = TRI->getEncodingValue(MO.getReg());
>> - RegIdx |= 1; //The odd register is also the higher-numbered one of
> a pair.
>> -
>> - unsigned Reg = RC.getRegister(RegIdx);
>> + unsigned Reg = MO.getReg();
>> + if(!ARM::GPRPairRegClass.contains(Reg))
>> + return false;
>> + Reg = TRI->getSubReg(Reg, ARM::gsub_1);
>> O << ARMInstPrinter::getRegisterName(Reg);
>> return false;
>> }
>>
>> Modified: llvm/trunk/lib/Target/ARM/ARMISelDAGToDAG.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMISelD
>> AGToDAG.cpp?rev=175088&r1=175087&r2=175088&view=diff
>> ======================================================================
>> ========
>> --- llvm/trunk/lib/Target/ARM/ARMISelDAGToDAG.cpp (original)
>> +++ llvm/trunk/lib/Target/ARM/ARMISelDAGToDAG.cpp Wed Feb 13 15:43:02
>> +++ 2013
>> @@ -19,6 +19,7 @@
>> #include "llvm/CodeGen/MachineFrameInfo.h"
>> #include "llvm/CodeGen/MachineFunction.h"
>> #include "llvm/CodeGen/MachineInstrBuilder.h"
>> +#include "llvm/CodeGen/MachineRegisterInfo.h"
>> #include "llvm/CodeGen/SelectionDAG.h"
>> #include "llvm/CodeGen/SelectionDAGISel.h"
>> #include "llvm/IR/CallingConv.h"
>> @@ -257,6 +258,8 @@ private:
>> // Select special operations if node forms integer ABS pattern
>> SDNode *SelectABSOp(SDNode *N);
>>
>> + SDNode *SelectInlineAsm(SDNode *N);
>> +
>> SDNode *SelectConcatVector(SDNode *N);
>>
>> SDNode *SelectAtomic64(SDNode *Node, unsigned Opc); @@ -2552,6
>> +2555,12 @@ SDNode *ARMDAGToDAGISel::Select(SDNode *
>>
>> switch (N->getOpcode()) {
>> default: break;
>> + case ISD::INLINEASM: {
>> + SDNode *ResNode = SelectInlineAsm(N);
>> + if (ResNode)
>> + return ResNode;
>> + break;
>> + }
>> case ISD::XOR: {
>> // Select special operations if XOR node forms integer ABS pattern
>> SDNode *ResNode = SelectABSOp(N); @@ -3446,6 +3455,138 @@ SDNode
>> *ARMDAGToDAGISel::Select(SDNode *
>> return SelectCode(N);
>> }
>>
>> +SDNode *ARMDAGToDAGISel::SelectInlineAsm(SDNode *N){
>> + std::vector<SDValue> AsmNodeOperands;
>> + unsigned Flag, Kind;
>> + bool Changed = false;
>> + unsigned NumOps = N->getNumOperands();
>> +
>> + ExternalSymbolSDNode *S = dyn_cast<ExternalSymbolSDNode>(
>> + N->getOperand(InlineAsm::Op_AsmString));
>> + StringRef AsmString = StringRef(S->getSymbol());
>> +
>> + // Normally, i64 data is bounded to two arbitrary GRPs for "%r"
> constraint.
>> + // However, some instrstions (e.g. ldrexd/strexd in ARM mode)
>> + require // (even/even+1) GPRs and use %n and %Hn to refer to the
>> + individual regs // respectively. Since there is no constraint to
>> + explicitly specify a // reg pair, we search %H operand inside the
>> + asm string. If it is found, the // transformation below enforces a
> GPRPair reg class for "%r" for 64-bit data.
>> + if (AsmString.find(":H}") == StringRef::npos)
>> + return NULL;
>> +
>> + DebugLoc dl = N->getDebugLoc();
>> + SDValue Glue = N->getOperand(NumOps-1);
>> +
>> + // Glue node will be appended late.
>> + for(unsigned i = 0; i < NumOps -1; ++i) {
>> + SDValue op = N->getOperand(i);
>> + AsmNodeOperands.push_back(op);
>> +
>> + if (i < InlineAsm::Op_FirstOperand)
>> + continue;
>> +
>> + if (ConstantSDNode *C = dyn_cast<ConstantSDNode>(N->getOperand(i))) {
>> + Flag = C->getZExtValue();
>> + Kind = InlineAsm::getKind(Flag);
>> + }
>> + else
>> + continue;
>> +
>> + if (Kind != InlineAsm::Kind_RegUse && Kind != InlineAsm::Kind_RegDef
>> + && Kind != InlineAsm::Kind_RegDefEarlyClobber)
>> + continue;
>> +
>> + unsigned RegNum = InlineAsm::getNumOperandRegisters(Flag);
>> + unsigned RC;
>> + bool HasRC = InlineAsm::hasRegClassConstraint(Flag, RC);
>> + if (!HasRC || RC != ARM::GPRRegClassID || RegNum != 2)
>> + continue;
>> +
>> + assert((i+2 < NumOps-1) && "Invalid number of operands in inline
> asm");
>> + SDValue V0 = N->getOperand(i+1);
>> + SDValue V1 = N->getOperand(i+2);
>> + unsigned Reg0 = cast<RegisterSDNode>(V0)->getReg();
>> + unsigned Reg1 = cast<RegisterSDNode>(V1)->getReg();
>> + SDValue PairedReg;
>> + MachineRegisterInfo &MRI = MF->getRegInfo();
>> +
>> + if (Kind == InlineAsm::Kind_RegDef ||
>> + Kind == InlineAsm::Kind_RegDefEarlyClobber) {
>> + // Replace the two GPRs with 1 GPRPair and copy values from GPRPair
> to
>> + // the original GPRs.
>> +
>> + unsigned GPVR = MRI.createVirtualRegister(&ARM::GPRPairRegClass);
>> + PairedReg = CurDAG->getRegister(GPVR, MVT::Untyped);
>> + SDValue Chain = SDValue(N,0);
>> +
>> + SDNode *GU = N->getGluedUser();
>> + SDValue RegCopy = CurDAG->getCopyFromReg(Chain, dl, GPVR,
> MVT::Untyped,
>> + Chain.getValue(1));
>> +
>> + // Extract values from a GPRPair reg and copy to the original GPR
> reg.
>> + SDValue Sub0 = CurDAG->getTargetExtractSubreg(ARM::gsub_0, dl,
> MVT::i32,
>> + RegCopy);
>> + SDValue Sub1 = CurDAG->getTargetExtractSubreg(ARM::gsub_1, dl,
> MVT::i32,
>> + RegCopy);
>> + SDValue T0 = CurDAG->getCopyToReg(Sub0, dl, Reg0, Sub0,
>> + RegCopy.getValue(1));
>> + SDValue T1 = CurDAG->getCopyToReg(Sub1, dl, Reg1, Sub1,
>> + T0.getValue(1));
>> +
>> + // Update the original glue user.
>> + std::vector<SDValue> Ops(GU->op_begin(), GU->op_end()-1);
>> + Ops.push_back(T1.getValue(1));
>> + CurDAG->UpdateNodeOperands(GU, &Ops[0], Ops.size());
>> + GU = T1.getNode();
>> + }
>> + else {
>> + // For Kind == InlineAsm::Kind_RegUse, we first copy two GPRs into
> a
>> + // GPRPair and then pass the GPRPair to the inline asm.
>> + SDValue Chain = AsmNodeOperands[InlineAsm::Op_InputChain];
>> +
>> + // As REG_SEQ doesn't take RegisterSDNode, we copy them first.
>> + SDValue T0 = CurDAG->getCopyFromReg(Chain, dl, Reg0, MVT::i32,
>> + Chain.getValue(1));
>> + SDValue T1 = CurDAG->getCopyFromReg(Chain, dl, Reg1, MVT::i32,
>> + T0.getValue(1));
>> + SDValue Pair = SDValue(createGPRPairNode(MVT::Untyped, T0, T1),
>> + 0);
>> +
>> + // Copy REG_SEQ into a GPRPair-typed VR and replace the original
> two
>> + // i32 VRs of inline asm with it.
>> + unsigned GPVR = MRI.createVirtualRegister(&ARM::GPRPairRegClass);
>> + PairedReg = CurDAG->getRegister(GPVR, MVT::Untyped);
>> + Chain = CurDAG->getCopyToReg(T1, dl, GPVR, Pair,
>> + T1.getValue(1));
>> +
>> + AsmNodeOperands[InlineAsm::Op_InputChain] = Chain;
>> + Glue = Chain.getValue(1);
>> + }
>> +
>> + Changed = true;
>> +
>> + if(PairedReg.getNode()) {
>> + Flag = InlineAsm::getFlagWord(Kind, 1 /* RegNum*/);
>> + Flag = InlineAsm::getFlagWordForRegClass(Flag,
> ARM::GPRPairRegClassID);
>> + // Replace the current flag.
>> + AsmNodeOperands[AsmNodeOperands.size() -1] =
> CurDAG->getTargetConstant(
>> + Flag, MVT::i32);
>> + // Add the new register node and skip the original two GPRs.
>> + AsmNodeOperands.push_back(PairedReg);
>> + // Skip the next two GPRs.
>> + i += 2;
>> + }
>> + }
>> +
>> + AsmNodeOperands.push_back(Glue);
>> + if (!Changed)
>> + return NULL;
>> +
>> + SDValue New = CurDAG->getNode(ISD::INLINEASM, N->getDebugLoc(),
>> + CurDAG->getVTList(MVT::Other, MVT::Glue), &AsmNodeOperands[0],
>> + AsmNodeOperands.size());
>> + New->setNodeId(-1);
>> + return New.getNode();
>> +}
>> +
>> +
>> bool ARMDAGToDAGISel::
>> SelectInlineAsmMemoryOperand(const SDValue &Op, char ConstraintCode,
>> std::vector<SDValue> &OutOps) {
>>
>> Modified: llvm/trunk/test/CodeGen/ARM/arm-modifier.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/arm-mo
>> difier.ll?rev=175088&r1=175087&r2=175088&view=diff
>> ======================================================================
>> ========
>> --- llvm/trunk/test/CodeGen/ARM/arm-modifier.ll (original)
>> +++ llvm/trunk/test/CodeGen/ARM/arm-modifier.ll Wed Feb 13 15:43:02
>> +++ 2013
>> @@ -61,8 +61,7 @@ ret void
>> define i64 @f4(i64* %val) nounwind {
>> entry:
>> ;CHECK: f4
>> - ;CHECK: ldrexd [[REG1:(r[0-9]?[02468])]], {{r[0-9]?[13579]}}, [r0]
>> - ;CHECK: mov r0, [[REG1]]
>> + ;CHECK: ldrexd [[REG1:(r[0-9]?[02468])]], {{r[0-9]?[13579]}},
>> + [r{{[0-9]+}}]
>> %0 = tail call i64 asm sideeffect "ldrexd $0, ${0:H}, [$1]",
> "=&r,r,*Qo"(i64* %val, i64* %val) nounwind
>> ret i64 %0
>> }
>>
>> Added: llvm/trunk/test/CodeGen/ARM/inlineasm-64bit.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/inline
>> asm-64bit.ll?rev=175088&view=auto
>> ======================================================================
>> ========
>> --- llvm/trunk/test/CodeGen/ARM/inlineasm-64bit.ll (added)
>> +++ llvm/trunk/test/CodeGen/ARM/inlineasm-64bit.ll Wed Feb 13 15:43:02
>> +++ 2013
>> @@ -0,0 +1,54 @@
>> +; RUN: llc < %s -O3 -march=arm | FileCheck %s
>> +
>> +; check if regs are passing correctly define void @i64_write(i64* %p,
>> +i64 %val) nounwind { ; CHECK: i64_write:
>> +; CHECK: ldrexd [[REG1:(r[0-9]?[02468])]], {{r[0-9]?[13579]}},
>> +[r{{[0-9]+}}] ; CHECK: strexd [[REG1]], {{r[0-9]?[02468]}},
>> +{{r[0-9]?[13579]}}
>> + %1 = tail call i64 asm sideeffect "1: ldrexd $0, ${0:H}, [$2]\0A
>> +strexd $0, $3, ${3:H}, [$2]\0A teq $0, #0\0A bne 1b",
>> +"=&r,=*Qo,r,r,~{cc}"(i64* %p, i64* %p, i64 %val) nounwind
>> + ret void
>> +}
>> +
>> +; check if register allocation can reuse the registers define void
>> + at multi_writes(i64* %p, i64 %val1, i64 %val2, i64 %val3, i64 %val4,
>> +i64 %val5, i64 %val6) nounwind {
>> +entry:
>> +; CHECK: multi_writes:
>> +; check: strexd {{r[0-9]?[02468]}}, {{r[0-9]?[13579]}}, [r{{[0-9]+}}]
>> +; check: strexd {{r[0-9]?[02468]}}, {{r[0-9]?[13579]}}, [r{{[0-9]+}}]
>> +; check: strexd {{r[0-9]?[02468]}}, {{r[0-9]?[13579]}}, [r{{[0-9]+}}]
>> +; check: strexd {{r[0-9]?[02468]}}, {{r[0-9]?[13579]}}, [r{{[0-9]+}}]
>> +; check: strexd {{r[0-9]?[02468]}}, {{r[0-9]?[13579]}}, [r{{[0-9]+}}]
>> +; check: strexd {{r[0-9]?[02468]}}, {{r[0-9]?[13579]}}, [r{{[0-9]+}}]
>> +
>> +; check: strexd {{r[0-9]?[02468]}}, {{r[0-9]?[13579]}}, [r{{[0-9]+}}]
>> +; check: strexd {{r[0-9]?[02468]}}, {{r[0-9]?[13579]}}, [r{{[0-9]+}}]
>> +; check: strexd {{r[0-9]?[02468]}}, {{r[0-9]?[13579]}}, [r{{[0-9]+}}]
>> +; check: strexd {{r[0-9]?[02468]}}, {{r[0-9]?[13579]}}, [r{{[0-9]+}}]
>> +; check: strexd {{r[0-9]?[02468]}}, {{r[0-9]?[13579]}}, [r{{[0-9]+}}]
>> +; check: strexd {{r[0-9]?[02468]}}, {{r[0-9]?[13579]}}, [r{{[0-9]+}}]
>> +
>> +; check: strexd {{r[0-9]?[02468]}}, {{r[0-9]?[13579]}}, [r{{[0-9]+}}]
>> +; check: strexd {{r[0-9]?[02468]}}, {{r[0-9]?[13579]}}, [r{{[0-9]+}}]
>> +; check: strexd {{r[0-9]?[02468]}}, {{r[0-9]?[13579]}}, [r{{[0-9]+}}]
>> +; check: strexd {{r[0-9]?[02468]}}, {{r[0-9]?[13579]}}, [r{{[0-9]+}}]
>> +; check: strexd {{r[0-9]?[02468]}}, {{r[0-9]?[13579]}}, [r{{[0-9]+}}]
>> +; check: strexd {{r[0-9]?[02468]}}, {{r[0-9]?[13579]}}, [r{{[0-9]+}}]
>> +
>> + tail call void asm sideeffect " strexd $1, ${1:H}, [$0]\0A strexd
>> +$2, ${2:H}, [$0]\0A strexd $3, ${3:H}, [$0]\0A strexd $4, ${4:H},
>> +[$0]\0A strexd $5, ${5:H}, [$0]\0A strexd $6, ${6:H}, [$0]\0A",
>> +"r,r,r,r,r,r,r"(i64* %p, i64 %val1, i64 %val2, i64 %val3, i64 %val4,
>> +i64 %val5, i64 %val6) nounwind
>> + %incdec.ptr = getelementptr inbounds i64* %p, i32 1
>> + tail call void asm sideeffect " strexd $1, ${1:H}, [$0]\0A strexd
>> +$2, ${2:H}, [$0]\0A strexd $3, ${3:H}, [$0]\0A strexd $4, ${4:H},
>> +[$0]\0A strexd $5, ${5:H}, [$0]\0A strexd $6, ${6:H}, [$0]\0A",
>> +"r,r,r,r,r,r,r"(i64* %incdec.ptr, i64 %val1, i64 %val2, i64 %val3,
>> +i64 %val4, i64 %val5, i64 %val6) nounwind
>> + tail call void asm sideeffect " strexd $1, ${1:H}, [$0]\0A strexd
>> +$2, ${2:H}, [$0]\0A strexd $3, ${3:H}, [$0]\0A strexd $4, ${4:H},
>> +[$0]\0A strexd $5, ${5:H}, [$0]\0A strexd $6, ${6:H}, [$0]\0A",
>> +"r,r,r,r,r,r,r"(i64* %incdec.ptr, i64 %val1, i64 %val2, i64 %val3,
>> +i64 %val4, i64 %val5, i64 %val6) nounwind
>> + ret void
>> +}
>> +
>> +
>> +; check if callee-saved registers used by inline asm are
>> +saved/restored define void @foo(i64* %p, i64 %i) nounwind { ;
>> +CHECK:foo:
>> +; CHECK: push {{{r[4-9]|r10|r11}}
>> +; CHECK: ldrexd [[REG1:(r[0-9]?[02468])]], {{r[0-9]?[13579]}},
>> +[r{{[0-9]+}}] ; CHECK: strexd [[REG1]], {{r[0-9]?[02468]}},
>> +{{r[0-9]?[13579]}} ; CHECK: pop {{{r[4-9]|r10|r11}}
>> + %1 = tail call { i64, i64 } asm sideeffect "@ atomic64_set\0A1:
>> +ldrexd $0, ${0:H}, [$3]\0Aldrexd $1, ${1:H}, [$3]\0A strexd $0, $4,
>> +${4:H}, [$3]\0A teq $0, #0\0A bne 1b", "=&r,=&r,=*Qo,r,r,~{cc}"(i64*
>> +%p, i64* %p, i64 %i) nounwind
>> + ret void
>> +}
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list