[llvm-commits] (patch) PR7608 ARMv4 JIT forgets to set the lr register when making a indirect function call
Xerxes Ranby
xerxes at zafena.se
Mon Jul 12 13:23:27 PDT 2010
Bob Wilson wrote:
> I don't think this is good enough. The "mov lr, pc" instruction must be emitted immediately before the call. I don't know how to do that without making all the "call_nolink" instructions into pseudo-instructions.
When writing the patch i had these references in mind:
http://wiki.llvm.org/Flags
http://www.nabble.com/-RFC%2C-ARM--expanding-RET-to-CopyToReg-BRIND-to4625955.html#a4632200
"Flag in the SelectionDAG stuff is so named because it was originally used
for condition codes. However, it has since grown to mean "keep these two
nodes always together". In the case of return, you want the scheduler to
produce code like this (on PPC):
...
R3 = outval_virtreg
blr
not like this:
...
R3 = outval_virtreg
...
blr
So the copy and blr are flagged together.
"
I have got the understanding that by flagging two instructions together
are the mechanism to use in order to prevent something-else from getting
scheduled in between.
Have this changed?
Where can I find documentation on what pseudo instructions are and how
pseudo instructions can resolve this kind of situation?
> That is kind of ugly because there are a fair number of those "call_nolink" instructions, but I can't think of anything better.
>
> (And, if it weren't for that blocking issue, I'd recommend changing the MOVLRPC instruction to explicitly define LR,
What do I have to do more than let Defs = [LR] in order to make MOVLRPC
explicitly define LR?
+let Defs = [LR], hasSideEffects = 1 in
+def MOVLRPC : AI<(outs), (ins), BrMiscFrm, IIC_Br,
+ "mov", "\tlr, pc", [(ARMmovlrpc)]>,
Cheers and have a great day!
Xerxes
> and changing all the "call_nolink" instructions to use LR. That would prevent LR from being clobbered between the MOVLRPC and the call. But, it's not good enough to prevent some other instruction being scheduled between them.....)
>
> On Jul 11, 2010, at 3:19 PM, Xerxes Ranby wrote:
>
>
>> http://llvm.org/bugs/show_bug.cgi?id=7608
>> Attached patch fixes the PR7608 bug by defining a new ARM selection dag node
>> ARMmovlrpc and makes sure both jit and non-jit got a ARMmovlrpc chained and
>> flagged before any ARMcall_nolink non-tailcall selection dag node.
>> * llvm/lib/Target/ARM/ARMISelLowering.cpp
>> (ARMTargetLowering::getTargetNodeName): Update to handle ARMISD::MOVLRPC.
>> (ARMTargetLowering::LowerCall): Add chain and flag a ARMmovlrpc before any
>> ARMcall_nolink non-tailcall selection dag node.
>> * llvm/lib/Target/ARM/ARMISelLowering.h
>> (ARMISD::MOVLRPC): New enum.
>> * llvm/lib/Target/ARM/ARMInstrInfo.td
>> (ARMmovlrpc): New defined selection dag node.
>> (MOVLRPC): New defined instruction.
>> (BX): Emit only one instruction for non-jit.
>> (BMOVPCRX): Likewise.
>> (BXr9): Likewise.
>> (BMOVPCRXr9): Likewise.
>>
>> Ok to push?
>>
>> Cheers
>> Xerxes
>>
>> Index: llvm/lib/Target/ARM/ARMISelLowering.cpp
>> ===================================================================
>> --- llvm.orig/lib/Target/ARM/ARMISelLowering.cpp 2010-07-11 12:57:59.000000000 +0200
>> +++ llvm/lib/Target/ARM/ARMISelLowering.cpp 2010-07-11 22:07:28.000000000 +0200
>> @@ -560,6 +560,7 @@
>> case ARMISD::BR_JT: return "ARMISD::BR_JT";
>> case ARMISD::BR2_JT: return "ARMISD::BR2_JT";
>> case ARMISD::RET_FLAG: return "ARMISD::RET_FLAG";
>> + case ARMISD::MOVLRPC: return "ARMISD::MOVLRPC";
>> case ARMISD::PIC_ADD: return "ARMISD::PIC_ADD";
>> case ARMISD::CMP: return "ARMISD::CMP";
>> case ARMISD::CMPZ: return "ARMISD::CMPZ";
>> @@ -1288,6 +1289,14 @@
>> // implicit def LR - LR mustn't be allocated as GRP:$dst of CALL_NOLINK
>> Chain = DAG.getCopyToReg(Chain, dl, ARM::LR, DAG.getUNDEF(MVT::i32),InFlag);
>> InFlag = Chain.getValue(1);
>> +
>> + if(!isTailCall){
>> + // explicit copy PC to LR and chain flag it to the call.
>> + Chain = DAG.getNode(ARMISD::MOVLRPC, dl,
>> + DAG.getVTList(MVT::Other, MVT::Flag),
>> + Chain, InFlag);
>> + InFlag = Chain.getValue(1);
>> + }
>> }
>>
>> std::vector<SDValue> Ops;
>> Index: llvm/lib/Target/ARM/ARMISelLowering.h
>> ===================================================================
>> --- llvm.orig/lib/Target/ARM/ARMISelLowering.h 2010-07-11 12:57:59.000000000 +0200
>> +++ llvm/lib/Target/ARM/ARMISelLowering.h 2010-07-11 12:59:02.000000000 +0200
>> @@ -42,6 +42,7 @@
>> BR_JT, // Jumptable branch.
>> BR2_JT, // Jumptable branch (2 level - jumptable entry is a jump).
>> RET_FLAG, // Return with a flag operand.
>> + MOVLRPC, // Store return address PC in LR before call - flag before CALL_NOLINK
>>
>> PIC_ADD, // Add with a PC operand and a PIC label.
>>
>> Index: llvm/lib/Target/ARM/ARMInstrInfo.td
>> ===================================================================
>> --- llvm.orig/lib/Target/ARM/ARMInstrInfo.td 2010-07-11 12:57:59.000000000 +0200
>> +++ llvm/lib/Target/ARM/ARMInstrInfo.td 2010-07-11 12:59:02.000000000 +0200
>> @@ -74,6 +74,9 @@
>> [SDNPHasChain, SDNPOptInFlag, SDNPOutFlag,
>> SDNPVariadic]>;
>>
>> +def ARMmovlrpc : SDNode<"ARMISD::MOVLRPC", SDTNone,
>> + [SDNPHasChain, SDNPOptInFlag, SDNPOutFlag]>;
>> +
>> def ARMretflag : SDNode<"ARMISD::RET_FLAG", SDTNone,
>> [SDNPHasChain, SDNPOptInFlag]>;
>>
>> @@ -674,6 +677,16 @@
>> [(ARMcallseq_start timm:$amt)]>;
>> }
>>
>> +let Defs = [LR], hasSideEffects = 1 in
>> +def MOVLRPC : AI<(outs), (ins), BrMiscFrm, IIC_Br,
>> + "mov", "\tlr, pc", [(ARMmovlrpc)]>,
>> + Requires<[IsARM]> {
>> + let Inst{11-0} = 0b000000001111;
>> + let Inst{15-12} = 0b1110;
>> + let Inst{19-16} = 0b0000;
>> + let Inst{27-20} = 0b00011010;
>> +}
>> +
>> def NOP : AI<(outs), (ins), MiscFrm, NoItinerary, "nop", "",
>> [/* For disassembly only; pattern left blank */]>,
>> Requires<[IsARM, HasV6T2]> {
>> @@ -962,7 +975,7 @@
>> // ARMv4T
>> // Note: Restrict $func to the tGPR regclass to prevent it being in LR.
>> def BX : ABXIx2<(outs), (ins tGPR:$func, variable_ops),
>> - IIC_Br, "mov\tlr, pc\n\tbx\t$func",
>> + IIC_Br, "bx\t$func",
>> [(ARMcall_nolink tGPR:$func)]>,
>> Requires<[IsARM, HasV4T, IsNotDarwin]> {
>> let Inst{7-4} = 0b0001;
>> @@ -972,7 +985,7 @@
>>
>> // ARMv4
>> def BMOVPCRX : ABXIx2<(outs), (ins tGPR:$func, variable_ops),
>> - IIC_Br, "mov\tlr, pc\n\tmov\tpc, $func",
>> + IIC_Br, "mov\tpc, $func",
>> [(ARMcall_nolink tGPR:$func)]>,
>> Requires<[IsARM, NoV4T, IsNotDarwin]> {
>> let Inst{11-4} = 0b00000000;
>> @@ -1011,7 +1024,7 @@
>> // ARMv4T
>> // Note: Restrict $func to the tGPR regclass to prevent it being in LR.
>> def BXr9 : ABXIx2<(outs), (ins tGPR:$func, variable_ops),
>> - IIC_Br, "mov\tlr, pc\n\tbx\t$func",
>> + IIC_Br, "bx\t$func",
>> [(ARMcall_nolink tGPR:$func)]>,
>> Requires<[IsARM, HasV4T, IsDarwin]> {
>> let Inst{7-4} = 0b0001;
>> @@ -1021,7 +1034,7 @@
>>
>> // ARMv4
>> def BMOVPCRXr9 : ABXIx2<(outs), (ins tGPR:$func, variable_ops),
>> - IIC_Br, "mov\tlr, pc\n\tmov\tpc, $func",
>> + IIC_Br, "mov\tpc, $func",
>> [(ARMcall_nolink tGPR:$func)]>,
>> Requires<[IsARM, NoV4T, IsDarwin]> {
>> let Inst{11-4} = 0b00000000;
>> _______________________________________________
>> 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