[llvm-commits] [llvm] r146492 - /llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp
Chad Rosier
mcrosier at apple.com
Tue Dec 13 10:18:55 PST 2011
On Dec 13, 2011, at 10:09 AM, Chris Lattner wrote:
>
> On Dec 13, 2011, at 9:45 AM, Chad Rosier wrote:
>
>> Author: mcrosier
>> Date: Tue Dec 13 11:45:06 2011
>> New Revision: 146492
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=146492&view=rev
>> Log:
>> [fast-isel] Remove SelectInsertValue() as fast-isel wasn't designed to handle
>> instructions that define aggregate types.
>
> Hi Chad,
>
> Are you sure that this isn't used? On X86-64, we do need to return aggregates to match the ABI. This is typically seen as a series of insertvalues right before the return. Is this simple case being handled still?
I implemented this last week, but Dan explained to me that fast-isel was never designed to support instructions that return an aggregate type (or rather define a series of registers). I believe UpdateValueMap wasn't designed to handle this case.
For example,
define void @test1() nounwind ssp {
%1 = insertvalue %struct.x undef, i32 1, 0
%2 = insertvalue %struct.x %1, i32 2, 1
call void @f(%struct.x %2) nounwind
ret void
}
For the above test case SelectInsertValue would generate the following machine instructions:
%vreg2<def> = MOVi16 2, pred:14, pred:%noreg; GPR:%vreg2
%vreg7<def> = MOVi16 1, pred:14, pred:%noreg; GPR:%vreg7
%vreg8<def> = COPY %vreg7; GPR:%vreg8,%vreg7
%vreg9<def> = IMPLICIT_DEF; GPR:%vreg9
%vreg5<def> = COPY %vreg8; GPR:%vreg5,%vreg8
%vreg6<def> = COPY %vreg2; GPR:%vreg6,%vreg2
ADJCALLSTACKDOWN 0, pred:14, pred:%noreg, %SP<imp-def,dead>, %SP<imp-use>
%R0<def> = COPY %vreg5; GPR:%vreg5
%R1<def> = COPY %vreg1; GPR:%vreg1
BLr9 <ga:@f>, %R0, %R1, %R0<imp-def,dead>, %R1<imp-def,dead>, %R7<imp-use>, %SP<imp-use>, ...
ADJCALLSTACKUP 0, 0, pred:14, pred:%noreg, %SP<imp-def,dead>, %SP<imp-use>
BX_RET pred:14, pred:%noreg
Notice %R1 is begin defined by a undefined virtual register, %vreg1.
%R1<def> = COPY %vreg1; GPR:%vreg1
It should be defined by %vreg6.
If this is something we really want I could discuss things with Dan.
Chad
> -Chris
>
>>
>> Modified:
>> llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp
>>
>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp?rev=146492&r1=146491&r2=146492&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp Tue Dec 13 11:45:06 2011
>> @@ -942,106 +942,6 @@
>> }
>>
>> bool
>> -FastISel::SelectInsertValue(const User *U) {
>> - return false;
Also note that I disabled this on Saturday after the nightly testers reported a bunch of miscompiles for the ARM testers.
Chad
>> - const InsertValueInst *IVI = dyn_cast<InsertValueInst>(U);
>> - if (!IVI)
>> - return false;
>> -
>> - // Only try to handle inserts of legal types. But also allow i16/i8/i1 because
>> - // they're easy.
>> - const Value *Val = IVI->getOperand(1);
>> - Type *ValTy = Val->getType();
>> - EVT ValVT = TLI.getValueType(ValTy, /*AllowUnknown=*/true);
>> - if (!ValVT.isSimple())
>> - return false;
>> - MVT VT = ValVT.getSimpleVT();
>> - if (!TLI.isTypeLegal(VT) && VT != MVT::i16 && VT != MVT::i8 && VT != MVT::i1)
>> - return false;
>> -
>> - // Get the Val register.
>> - unsigned ValReg = getRegForValue(Val);
>> - if (ValReg == 0) return false;
>> -
>> - const Value *Agg = IVI->getOperand(0);
>> - Type *AggTy = Agg->getType();
>> -
>> - // TODO: Is there a better way to do this? For each insertvalue we allocate
>> - // a new set of virtual registers, which results in a large number of
>> - // loads/stores from/to the stack that copies the aggregate all over the place
>> - // and results in lots of spill code. I believe this is necessary to preserve
>> - // SSA form, but maybe there's something we could do to improve this.
>> -
>> - // Get the Aggregate base register.
>> - unsigned AggBaseReg;
>> - DenseMap<const Value *, unsigned>::iterator I = FuncInfo.ValueMap.find(Agg);
>> - if (I != FuncInfo.ValueMap.end())
>> - AggBaseReg = I->second;
>> - else if (isa<Instruction>(Agg))
>> - AggBaseReg = FuncInfo.InitializeRegForValue(Agg);
>> - else if (isa<UndefValue>(Agg))
>> - // In this case we don't need to allocate a new set of register that will
>> - // never be defined. Just copy Val into the proper result registers.
>> - AggBaseReg = 0;
>> - else
>> - return false; // fast-isel can't handle aggregate constants at the moment
>> -
>> - // Create result register(s).
>> - unsigned ResultBaseReg = FuncInfo.CreateRegs(AggTy);
>> -
>> - // Get the actual result register, which is an offset from the base register.
>> - unsigned LinearIndex = ComputeLinearIndex(Agg->getType(), IVI->getIndices());
>> -
>> - SmallVector<EVT, 4> AggValueVTs;
>> - ComputeValueVTs(TLI, AggTy, AggValueVTs);
>> -
>> - // Copy the beginning value(s) from the original aggregate.
>> - unsigned SrcReg;
>> - unsigned DestReg;
>> - unsigned BaseRegOff = 0;
>> - unsigned i = 0;
>> - for (; i != LinearIndex; ++i) {
>> - unsigned NRE = TLI.getNumRegisters(FuncInfo.Fn->getContext(),
>> - AggValueVTs[i]);
>> - for (unsigned NRI = 0; NRI != NRE; NRI++) {
>> - if (AggBaseReg) {
>> - SrcReg = AggBaseReg + BaseRegOff + NRI;
>> - DestReg = ResultBaseReg + BaseRegOff + NRI;
>> - BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DL, TII.get(TargetOpcode::COPY),
>> - DestReg).addReg(SrcReg);
>> - }
>> - }
>> - BaseRegOff += NRE;
>> - }
>> -
>> - // FIXME: Handle aggregate inserts. Haven't seen these in practice, but..
>> - // Copy value(s) from the inserted value(s).
>> - DestReg = ResultBaseReg + BaseRegOff;
>> - BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DL, TII.get(TargetOpcode::COPY),
>> - DestReg).addReg(ValReg);
>> - ++BaseRegOff;
>> - ++i;
>> -
>> - // Copy remaining value(s) from the original aggregate.
>> - if (AggBaseReg) {
>> - for (unsigned NumAggValues = AggValueVTs.size(); i != NumAggValues; ++i) {
>> - unsigned NRE = TLI.getNumRegisters(FuncInfo.Fn->getContext(),
>> - AggValueVTs[i]);
>> - for (unsigned NRI = 0; NRI != NRE; NRI++) {
>> - SrcReg = AggBaseReg + BaseRegOff + NRI;
>> - DestReg = ResultBaseReg + BaseRegOff + NRI;
>> - BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DL, TII.get(TargetOpcode::COPY),
>> - DestReg).addReg(SrcReg);
>> -
>> - }
>> - BaseRegOff += NRE;
>> - }
>> - }
>> - UpdateValueMap(IVI, ResultBaseReg);
>> - return true;
>> -}
>> -
>> -bool
>> FastISel::SelectOperator(const User *I, unsigned Opcode) {
>> switch (Opcode) {
>> case Instruction::Add:
>> @@ -1148,9 +1048,6 @@
>> case Instruction::ExtractValue:
>> return SelectExtractValue(I);
>>
>> - case Instruction::InsertValue:
>> - return SelectInsertValue(I);
>> -
>> case Instruction::PHI:
>> llvm_unreachable("FastISel shouldn't visit PHI nodes!");
>>
>>
>>
>> _______________________________________________
>> 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