[llvm-commits] (patch) PR7608 ARMv4 JIT forgets to set the lr register when making a indirect function call
Bob Wilson
bob.wilson at apple.com
Mon Jul 12 13:35:05 PDT 2010
On Jul 12, 2010, at 1:23 PM, Xerxes Ranby wrote:
> 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?
No, I'm not aware that it has changed recently, but flagging and chaining only work at the SelectionDAG level. Once the code gets selected to MachineInstrs, there's nothing to keep things from moving around.
>
> Where can I find documentation on what pseudo instructions are and how pseudo instructions can resolve this kind of situation?
Look at ARMCodeEmitter::emitPseudoInstruction for some examples. Basically you would just change the "call_nolink" instructions to be marked as pseudo instructions and then add code to ARMCodeEmitter to generate both the "mov lr, pc" and "bx" instructions together.
>> 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)]>,
Oh, I guess I missed that. You would still need to make the "call_nolink" instruction use LR, but it's not really relevant because it won't solve the problem of keeping the mov/bx together.
>
>
> 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