[llvm-commits] [llvm] r56290 - in /llvm/trunk: include/llvm/CodeGen/LiveIntervalAnalysis.h include/llvm/CodeGen/MachineOperand.h include/llvm/CodeGen/ScheduleDAG.h lib/CodeGen/AsmPrinter/AsmPrinter.cpp lib/CodeGen/LiveIntervalAnalysis.cpp lib/CodeGen/MachineInstr.cpp lib/CodeGen/RegAllocLinearScan.cpp lib/CodeGen/SelectionDAG/ScheduleDAGEmit.cpp lib/CodeGen/SelectionDAG/SelectionDAGBuild.cpp lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp test/CodeGen/X86/2008-09-17-inline-asm-1.ll
Evan Cheng
evan.cheng at apple.com
Thu Sep 18 10:17:04 PDT 2008
Hi Dale,
Thanks for doing this. Comments inline.
On Sep 17, 2008, at 2:13 PM, Dale Johannesen wrote:
LiveIntervals::releaseMemory() should release
AsmsWithEarlyClobberConflict and AsmsThatEarlyClobber, no?
>
> +/// noEarlyclobberConflict - see whether virtual reg VReg has a
> conflict with
> +/// hard reg HReg because of earlyclobbers.
> +///
> +/// Earlyclobber operands may not be assigned the same register as
> +/// each other, or as earlyclobber-conflict operands (i.e. those that
> +/// are non-earlyclobbered inputs to an asm that also has
> earlyclobbers).
> +///
> +/// Thus there are two cases to check for:
> +/// 1. VReg is an earlyclobber-conflict register and HReg is an
> earlyclobber
> +/// register in some asm that also has VReg as an input.
> +/// 2. VReg is an earlyclobber register and HReg is an
> earlyclobber-conflict
> +/// input elsewhere in some asm.
> +/// In both cases HReg can be assigned by the user, or assigned
> early in
> +/// register allocation.
> +///
> +/// Dropping the distinction between earlyclobber and earlyclobber-
> conflict,
> +/// keeping only one multimap, looks promising, but two
> earlyclobber-conflict
> +/// operands may be assigned the same register if they happen to
> contain the
> +/// same value, and that implementation would prevent this.
> +///
> +bool LiveIntervals::noEarlyclobberConflict(unsigned VReg,
> VirtRegMap &vrm,
> + unsigned HReg) {
> + typedef std::multimap<unsigned, MachineInstr*>::iterator It;
> +
> + // Short circuit the most common case.
> + if (AsmsWithEarlyClobberConflict.size()!=0) {
If AsmsWithEarlyClobberConflict is empty, there is no need to check
for AsmsThatEarlyClobber, right? And vice versa.
Also, I don't think this is a safe approach, machine instructions can
be deleted and leave the maps in the invalid state.
>
> + std::pair<It, It> x =
> AsmsWithEarlyClobberConflict.equal_range(VReg);
> + for (It I = x.first; I!=x.second; I++) {
> + MachineInstr* MI = I->second;
> + for (int i = MI->getNumOperands() - 1; i >= 0; --i) {
> + MachineOperand &MO = MI->getOperand(i);
> + if (MO.isRegister() && MO.isEarlyClobber()) {
> + unsigned PhysReg = MO.getReg();
> + if (PhysReg &&
> TargetRegisterInfo::isVirtualRegister(PhysReg)) {
> + if (!vrm.hasPhys(PhysReg))
> + continue;
> + PhysReg = vrm.getPhys(PhysReg);
> + }
> + if (PhysReg==HReg)
> + return false;
> + }
> + }
> + }
> + }
> + // Short circuit the most common case.
> + if (AsmsThatEarlyClobber.size()!=0) {
> + std::pair<It, It> x = AsmsThatEarlyClobber.equal_range(VReg);
> + for (It I = x.first; I!=x.second; I++) {
> + MachineInstr* MI = I->second;
> + for (int i = MI->getNumOperands() - 1; i >= 0; --i) {
> + MachineOperand &MO = MI->getOperand(i);
> + if (MO.isRegister() && MO.overlapsEarlyClobber()) {
> + unsigned PhysReg = MO.getReg();
> + if (PhysReg &&
> TargetRegisterInfo::isVirtualRegister(PhysReg)) {
> + if (!vrm.hasPhys(PhysReg))
> + continue;
> + PhysReg = vrm.getPhys(PhysReg);
> + }
> + if (PhysReg==HReg)
> + return false;
> + }
> + }
> + }
> + }
> + return true;
> +}
>
> LiveInterval* LiveIntervals::createInterval(unsigned reg) {
> float Weight = TargetRegisterInfo::isPhysicalRegister(reg) ?
>
>
> Modified: llvm/trunk/lib/CodeGen/RegAllocLinearScan.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/RegAllocLinearScan.cpp?rev=56290&r1=56289&r2=56290&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/lib/CodeGen/RegAllocLinearScan.cpp (original)
> +++ llvm/trunk/lib/CodeGen/RegAllocLinearScan.cpp Wed Sep 17
> 16:13:11 2008
> @@ -1050,7 +1050,8 @@
> TargetRegisterClass::iterator E = RC->allocation_order_end(*mf_);
> assert(I != E && "No allocatable register in this register class!");
> for (; I != E; ++I)
> - if (prt_->isRegAvail(*I)) {
> + if (prt_->isRegAvail(*I) &&
> + li_->noEarlyclobberConflict(cur->reg, *vrm_, *I)) {
Ok, I have two concerns. 1. We need to do this check for every
interval. 2. After AsmsThatEarlyClobber and
AsmsWithEarlyClobberConflict are populated virtual registers can
change due to coalescing (and potentially other passes). Then the maps
won't be accurate.
Perhaps we should add a bit to LiveInterval to indicate a vr can be
earlyclobber or earlyclobberconflict. Coalescer should not touch any
register whose interval has the bit. Then we only do the check for
those intervals here. The check can use MachineRegisterInfo def / use
chain to look for any conflicts.
Evan
>
> FreeReg = *I;
> if (FreeReg < inactiveCounts.size())
> FreeRegInactiveCount = inactiveCounts[FreeReg];
> @@ -1070,7 +1071,8 @@
> for (; I != E; ++I) {
> unsigned Reg = *I;
> if (prt_->isRegAvail(Reg) && Reg < inactiveCounts.size() &&
> - FreeRegInactiveCount < inactiveCounts[Reg]) {
> + FreeRegInactiveCount < inactiveCounts[Reg] &&
> + li_->noEarlyclobberConflict(cur->reg, *vrm_, Reg)) {
> FreeReg = Reg;
> FreeRegInactiveCount = inactiveCounts[Reg];
> if (FreeRegInactiveCount == MaxInactiveCount)
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGEmit.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGEmit.cpp?rev=56290&r1=56289&r2=56290&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGEmit.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGEmit.cpp Wed Sep
> 17 16:13:11 2008
> @@ -231,7 +231,8 @@
> void ScheduleDAG::AddOperand(MachineInstr *MI, SDValue Op,
> unsigned IIOpNum,
> const TargetInstrDesc *II,
> - DenseMap<SDValue, unsigned>
> &VRBaseMap) {
> + DenseMap<SDValue, unsigned> &VRBaseMap,
> + bool overlapsEarlyClobber) {
> if (Op.isMachineOpcode()) {
> // Note that this case is redundant with the final else block,
> but we
> // include it because it is the most common and it makes the logic
> @@ -244,7 +245,9 @@
> const TargetInstrDesc &TID = MI->getDesc();
> bool isOptDef = IIOpNum < TID.getNumOperands() &&
> TID.OpInfo[IIOpNum].isOptionalDef();
> - MI->addOperand(MachineOperand::CreateReg(VReg, isOptDef));
> + MI->addOperand(MachineOperand::CreateReg(VReg, isOptDef, false,
> false,
> + false, 0, false,
> + overlapsEarlyClobber));
>
> // Verify that it is right.
> assert(TargetRegisterInfo::isVirtualRegister(VReg) && "Not a
> vreg?");
> @@ -278,7 +281,9 @@
> const ConstantFP *CFP = F->getConstantFPValue();
> MI->addOperand(MachineOperand::CreateFPImm(CFP));
> } else if (RegisterSDNode *R = dyn_cast<RegisterSDNode>(Op)) {
> - MI->addOperand(MachineOperand::CreateReg(R->getReg(), false));
> + MI->addOperand(MachineOperand::CreateReg(R->getReg(), false,
> false,
> + false, false, 0, false,
> + overlapsEarlyClobber));
> } else if (GlobalAddressSDNode *TGA =
> dyn_cast<GlobalAddressSDNode>(Op)) {
> MI->addOperand(MachineOperand::CreateGA(TGA->getGlobal(),TGA-
> >getOffset()));
> } else if (BasicBlockSDNode *BB = dyn_cast<BasicBlockSDNode>(Op)) {
> @@ -314,7 +319,9 @@
> Op.getValueType() != MVT::Flag &&
> "Chain and flag operands should occur at end of operand
> list!");
> unsigned VReg = getVR(Op, VRBaseMap);
> - MI->addOperand(MachineOperand::CreateReg(VReg, false));
> + MI->addOperand(MachineOperand::CreateReg(VReg, false, false,
> + false, false, 0, false,
> + overlapsEarlyClobber));
>
> // Verify that it is right. Note that the reg class of the
> physreg and the
> // vreg don't necessarily need to match, but the target copy
> insertion has
> @@ -596,6 +603,7 @@
>
> // Add all of the operand registers to the instruction.
> for (unsigned i = 2; i != NumOps;) {
> + bool overlapsEarlyClobber = false;
> unsigned Flags =
> cast<ConstantSDNode>(Node->getOperand(i))->getZExtValue();
> unsigned NumVals = Flags >> 3;
> @@ -618,13 +626,18 @@
> false, 0, true));
> }
> break;
> + case 7: // Addressing mode overlapping earlyclobber.
> + case 5: // Use of register overlapping earlyclobber.
> + overlapsEarlyClobber = true;
> + // fall through
> case 1: // Use of register.
> case 3: // Immediate.
> case 4: // Addressing mode.
> // The addressing mode has been selected, just add all of the
> // operands to the machine instruction.
> for (; NumVals; --NumVals, ++i)
> - AddOperand(MI, Node->getOperand(i), 0, 0, VRBaseMap);
> + AddOperand(MI, Node->getOperand(i), 0, 0, VRBaseMap,
> + overlapsEarlyClobber);
> break;
> }
> }
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuild.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuild.cpp?rev=56290&r1=56289&r2=56290&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuild.cpp
> (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuild.cpp Wed
> Sep 17 16:13:11 2008
> @@ -4909,8 +4909,10 @@
> assert(OpInfo.isIndirect && "Memory output must be indirect
> operand");
>
> // Add information to the INLINEASM node to know about this
> output.
> - unsigned ResOpType = 4/*MEM*/ | (1 << 3);
> - AsmNodeOperands.push_back(DAG.getTargetConstant(ResOpType,
> + unsigned ResOpType = SawEarlyClobber ?
> + 7 /* MEM OVERLAPS EARLYCLOBBER */ :
> + 4/*MEM*/;
> + AsmNodeOperands.push_back(DAG.getTargetConstant(ResOpType |
> (1<<3),
>
> TLI.getPointerTy()));
> AsmNodeOperands.push_back(OpInfo.CallOperand);
> break;
> @@ -4963,7 +4965,8 @@
> cast<ConstantSDNode>(AsmNodeOperands[CurOp])-
> >getZExtValue();
> assert(((NumOps & 7) == 2 /*REGDEF*/ ||
> (NumOps & 7) == 6 /*EARLYCLOBBER REGDEF*/ ||
> - (NumOps & 7) == 4 /*MEM*/) &&
> + (NumOps & 7) == 4 /*MEM*/ ||
> + (NumOps & 7) == 7 /*MEM OVERLAPS EARLYCLOBBER*/) &&
> "Skipped past definitions?");
> CurOp += (NumOps>>3)+1;
> }
> @@ -4985,14 +4988,17 @@
>
> // Use the produced MatchedRegs object to
> MatchedRegs.getCopyToRegs(InOperandVal, DAG, Chain, &Flag);
> - MatchedRegs.AddInlineAsmOperands(1 /*REGUSE*/, DAG,
> AsmNodeOperands);
> + MatchedRegs.AddInlineAsmOperands(SawEarlyClobber ?
> + 1 /*REGUSE*/ :
> + 5 /*REGUSE OVERLAPS
> EARLYCLOBBER*/,
> + DAG, AsmNodeOperands);
> break;
> } else {
> - assert((NumOps & 7) == 4/*MEM*/ && "Unknown matching
> constraint!");
> + assert(((NumOps & 7) == 7/*MEM OVERLAPS EARLYCLOBBER */ ||
> + (NumOps & 7) == 4) && "Unknown matching
> constraint!");
> assert((NumOps >> 3) == 1 && "Unexpected number of
> operands");
> // Add information to the INLINEASM node to know about
> this input.
> - unsigned ResOpType = 4/*MEM*/ | (1 << 3);
> - AsmNodeOperands.push_back(DAG.getTargetConstant(ResOpType,
> + AsmNodeOperands.push_back(DAG.getTargetConstant(NumOps,
>
> TLI.getPointerTy()));
> AsmNodeOperands.push_back(AsmNodeOperands[CurOp+1]);
> break;
> @@ -5024,8 +5030,10 @@
> "Memory operands expect pointer values");
>
> // Add information to the INLINEASM node to know about this
> input.
> - unsigned ResOpType = 4/*MEM*/ | (1 << 3);
> - AsmNodeOperands.push_back(DAG.getTargetConstant(ResOpType,
> + unsigned ResOpType = SawEarlyClobber ?
> + 7 /* MEM OVERLAPS EARLYCLOBBER */ :
> + 4/*MEM*/;
> + AsmNodeOperands.push_back(DAG.getTargetConstant(ResOpType |
> (1<<3),
>
> TLI.getPointerTy()));
> AsmNodeOperands.push_back(InOperandVal);
> break;
> @@ -5043,16 +5051,18 @@
>
> OpInfo.AssignedRegs.getCopyToRegs(InOperandVal, DAG, Chain,
> &Flag);
>
> - OpInfo.AssignedRegs.AddInlineAsmOperands(1/*REGUSE*/, DAG,
> - AsmNodeOperands);
> + OpInfo.AssignedRegs.AddInlineAsmOperands(SawEarlyClobber ?
> + 5 /*REGUSE OVERLAPS
> EARLYCLOBBER*/:
> + 1/*REGUSE*/,
> + DAG, AsmNodeOperands);
> break;
> }
> case InlineAsm::isClobber: {
> // Add the clobbered value to the operand list, so that the
> register
> // allocator is aware that the physreg got clobbered.
> if (!OpInfo.AssignedRegs.Regs.empty())
> - OpInfo.AssignedRegs.AddInlineAsmOperands(2/*REGDEF*/, DAG,
> - AsmNodeOperands);
> + OpInfo.AssignedRegs.AddInlineAsmOperands(6 /* EARLYCLOBBER
> REGDEF */,
> + DAG,
> AsmNodeOperands);
> break;
> }
> }
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp?rev=56290&r1=56289&r2=56290&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
> (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp Wed Sep
> 17 16:13:11 2008
> @@ -1113,7 +1113,8 @@
>
> while (i != e) {
> unsigned Flags = cast<ConstantSDNode>(InOps[i])->getZExtValue();
> - if ((Flags & 7) != 4 /*MEM*/) {
> + if ((Flags & 7) != 4 /*MEM*/ &&
> + (Flags & 7) != 7 /*MEM OVERLAPS EARLYCLOBBER*/) {
> // Just skip over this operand, copying the operands verbatim.
> Ops.insert(Ops.end(), InOps.begin()+i, InOps.begin()+i+(Flags
> >> 3) + 1);
> i += (Flags >> 3) + 1;
> @@ -1128,7 +1129,7 @@
>
> // Add this to the output node.
> MVT IntPtrTy = CurDAG->getTargetLoweringInfo().getPointerTy();
> - Ops.push_back(CurDAG->getTargetConstant(4/*MEM*/ |
> (SelOps.size() << 3),
> + Ops.push_back(CurDAG->getTargetConstant((Flags & 7) |
> (SelOps.size()<< 3),
> IntPtrTy));
> Ops.insert(Ops.end(), SelOps.begin(), SelOps.end());
> i += 2;
>
> Added: llvm/trunk/test/CodeGen/X86/2008-09-17-inline-asm-1.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/2008-09-17-inline-asm-1.ll?rev=56290&view=auto
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/test/CodeGen/X86/2008-09-17-inline-asm-1.ll (added)
> +++ llvm/trunk/test/CodeGen/X86/2008-09-17-inline-asm-1.ll Wed Sep
> 17 16:13:11 2008
> @@ -0,0 +1,24 @@
> +; RUN: llvm-as < %s | llc -o - -march=x86 | not grep "movl %eax,
> %eax"
> +; RUN: llvm-as < %s | llc -o - -march=x86 | not grep "movl %edx,
> %edx"
> +; RUN: llvm-as < %s | llc -o - -march=x86 | not grep "movl (%eax),
> %eax"
> +; RUN: llvm-as < %s | llc -o - -march=x86 | not grep "movl (%edx),
> %edx"
> +
> +; %0 must not be put in EAX or EDX.
> +; In the first asm, $0 and $2 must not be put in EAX.
> +; In the second asm, $0 and $2 must not be put in EDX.
> +; This is kind of hard to test thoroughly, but the things above
> should continue
> +; to pass, I think.
> +; ModuleID = '<stdin>'
> +target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-
> i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-
> f80:128:128"
> +target triple = "i386-apple-darwin8"
> + at x = common global i32 0 ; <i32*> [#uses=1]
> +
> +define i32 @aci(i32* %pw) nounwind {
> +entry:
> + %0 = load i32* @x, align 4 ; <i32> [#uses=1]
> + %asmtmp = tail call { i32, i32 } asm "movl $0, %eax\0A\090:\0A
> \09test %eax, %eax\0A\09je 1f\0A\09movl %eax, $2\0A\09incl $2\0A
> \09lock\0A\09cmpxchgl $2, $0\0A\09jne 0b\0A\091:",
> "=*m,=&{ax},=&r,*m,~{dirflag},~{fpsr},~{flags},~{memory},~{cc}"(i32*
> %pw, i32* %pw) nounwind ; <{ i32, i32 }> [#uses=0]
> + %asmtmp2 = tail call { i32, i32 } asm "movl $0, %edx\0A\090:\0A
> \09test %edx, %edx\0A\09je 1f\0A\09movl %edx, $2\0A\09incl $2\0A
> \09lock\0A\09cmpxchgl $2, $0\0A\09jne 0b\0A\091:",
> "=*m,=&{dx},=&r,*m,~{dirflag},~{fpsr},~{flags},~{memory},~{cc}"(i32*
> %pw, i32* %pw) nounwind ; <{ i32, i32 }> [#uses=1]
> + %asmresult3 = extractvalue { i32, i32 } %asmtmp2, 0 ; <i32>
> [#uses=1]
> + %1 = add i32 %asmresult3, %0 ; <i32> [#uses=1]
> + ret i32 %1
> +}
>
>
> _______________________________________________
> 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