[LLVMdev] Using CallingConvLower in ARM target
Evan Cheng
evan.cheng at apple.com
Mon Feb 16 11:00:14 PST 2009
Thanks.
More questions :-)
/// Information about how the value is assigned.
- LocInfo HTP : 7;
+ LocInfo HTP : 6;
Do you know why this change is needed? Are we running out of bits?
- NeededStackSize = 4;
- break;
- case MVT::i64:
- case MVT::f64:
- if (firstGPR < 3)
- NeededGPRs = 2;
- else if (firstGPR == 3) {
- NeededGPRs = 1;
- NeededStackSize = 4;
- } else
- NeededStackSize = 8;
+ State.addLoc(CCValAssign::getCustomMem(ValNo, ValVT,
+ State.AllocateStack(4, 4),
+ MVT::i32, LocInfo));
+ return true; // we handled it
Your change isn't handling the "NeededStackSize = 8" case.
++ static const unsigned HiRegList[] = { ARM::R0, ARM::R2 };
+ static const unsigned LoRegList[] = { ARM::R1, ARM::R3 };
+
+ if (unsigned Reg = State.AllocateReg(HiRegList, LoRegList, 2)) {
+ unsigned i;
+ for (i = 0; i < 2; ++i)
+ if (HiRegList[i] == Reg)
+ break;
+
+ State.addLoc(CCValAssign::getCustomReg(ValNo, ValVT, Reg,
+ MVT::i32, LocInfo));
+ State.addLoc(CCValAssign::getCustomReg(ValNo, ValVT, LoRegList[i],
+ MVT::i32, LocInfo));
Since 'i' is used after the loop, please choose a better variable name.
Actually, is the loop necessary? We know the low register is always
one after the high register. Perhaps you can use
ARMRegisterInfo::getRegisterNumbering(Reg), add one to 1. And the
lookup the register enum with a new function (something like
getRegFromRegisterNum(RegNo, ValVT)).
The patch is looking good. I need to run it through some more tests.
Unfortunately ARM target is a bit broken right now. I hope to fix it
today.
Thanks,
Evan
On Feb 13, 2009, at 8:27 PM, Sandeep Patel wrote:
> Sorry left a small bit of cruft in ARMCallingConv.td. A corrected
> patch it attached.
>
> deep
>
> On Fri, Feb 13, 2009 at 6:41 PM, Sandeep Patel <deeppatel1987 at gmail.com
> > wrote:
>> Sure. Updated patches attached.
>>
>> deep
>>
>> On Fri, Feb 13, 2009 at 5:47 PM, Evan Cheng <evan.cheng at apple.com>
>> wrote:
>>>
>>> On Feb 13, 2009, at 4:25 PM, Sandeep Patel wrote:
>>>
>>>> ARMTargetLowering doesn't need case #1, but it seemed like you
>>>> and Dan
>>>> wanted a more generic way to inject C++ code into the process so I
>>>> tried to make the mechanism a bit more general.
>>>
>>> Ok. Since ARM doesn't need it and it's the only client, I'd much
>>> rather have CCCustomFn just return a single bool indicating
>>> whether it
>>> can handle the arg. Would that be ok?
>>>
>>> Thanks,
>>>
>>> Evan
>>>
>>>>
>>>>
>>>> deep
>>>>
>>>> On Fri, Feb 13, 2009 at 2:34 PM, Evan Cheng <evan.cheng at apple.com>
>>>> wrote:
>>>>>
>>>>> On Feb 13, 2009, at 2:20 PM, Sandeep Patel wrote:
>>>>>
>>>>>> On Fri, Feb 13, 2009 at 12:33 PM, Evan Cheng <evan.cheng at apple.com
>>>>>> >
>>>>>> wrote:
>>>>>>>
>>>>>>> On Feb 12, 2009, at 6:21 PM, Sandeep Patel wrote:
>>>>>>>
>>>>>>>> Although it's not generally needed for ARM's use of CCCustom, I
>>>>>>>> return
>>>>>>>> two bools to handle the four possible outcomes to keep the
>>>>>>>> mechanism
>>>>>>>> flexible:
>>>>>>>>
>>>>>>>> * if CCCustomFn handled the arg or not
>>>>>>>> * if CCCustomFn wants to end processing of the arg or not
>>>>>>>
>>>>>>> +/// CCCustomFn - This function assigns a location for Val,
>>>>>>> possibly
>>>>>>> updating
>>>>>>> +/// all args to reflect changes and indicates if it handled
>>>>>>> it. It
>>>>>>> must set
>>>>>>> +/// isCustom if it handles the arg and returns true.
>>>>>>> +typedef bool CCCustomFn(unsigned &ValNo, MVT &ValVT,
>>>>>>> + MVT &LocVT, CCValAssign::LocInfo
>>>>>>> &LocInfo,
>>>>>>> + ISD::ArgFlagsTy &ArgFlags, CCState
>>>>>>> &State,
>>>>>>> + bool &result);
>>>>>>>
>>>>>>> Is "result" what you refer to as "isCustom" in the comments?
>>>>>>>
>>>>>>> Sorry, I am still confused. You mean it could return true but
>>>>>>> set
>>>>>>> 'result' to false? That means it has handled the argument but it
>>>>>>> would
>>>>>>> not process any more arguments? What scenario do you envision
>>>>>>> that
>>>>>>> this will be useful? I'd rather keep it simple.
>>>>>>
>>>>>> As you note there are three actual legitimate cases (of the four
>>>>>> combos):
>>>>>>
>>>>>> 1. The CCCustomFn wants the arg handling to proceed. This might
>>>>>> be
>>>>>> used akin to CCPromoteToType.
>>>>>> 2. The CCCustomFn entirely handled the arg. This might be used
>>>>>> akin to
>>>>>> CCAssignToReg.
>>>>>> 3. The CCCustomFn tried to handle the arg, but failed.
>>>>>>
>>>>>> these results are conveyed the following ways:
>>>>>>
>>>>>> 1. The CCCustomFn returns false, &result is not used.
>>>>>> 2. The CCCustomFn returns true, &result is false;
>>>>>> 3. The CCCustomFn returns true, &result is true.
>>>>>
>>>>> I don't think we want to support #1. If the target want to add
>>>>> custom
>>>>> code to handle an argument, if should be responsible for
>>>>> outputting
>>>>> legal code. Is there an actual need to support #1?
>>>>>
>>>>> Evan
>>>>>
>>>>>>
>>>>>>
>>>>>> I tried to keep these CCCustomFns looking like TableGen generated
>>>>>> code. Suggestions of how to reorganize these results are
>>>>>> welcome. :-)
>>>>>> Perhaps better comments around the typedef for CCCustomFn would
>>>>>> suffice?
>>>>>>
>>>>>> The isCustom flag is simply a means for this machinery to
>>>>>> convey to
>>>>>> the TargetLowering functions to process this arg specially. It
>>>>>> may
>>>>>> not
>>>>>> always be possible for the TargetLowering functions to determine
>>>>>> that
>>>>>> the arg needs special handling after all the changes made by the
>>>>>> CCCustomFn or CCPromoteToType and other transformations.
>>>>>>
>>>>>>>> I placed the "unsigned i" outside those loops because i is used
>>>>>>>> after
>>>>>>>> the loop. If there's a better index search pattern, I'd be
>>>>>>>> happy
>>>>>>>> to
>>>>>>>> change it.
>>>>>>>
>>>>>>> Ok.
>>>>>>>
>>>>>>> One more nitpick:
>>>>>>>
>>>>>>> +/// CCCustom - calls a custom arg handling function
>>>>>>>
>>>>>>> Please capitalize "calls" and end with a period.
>>>>>>
>>>>>> Once we settle on the result handling changes, I'll submit an
>>>>>> update
>>>>>> with this change.
>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Evan
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Attached is an updated patch against HEAD that has DebugLoc
>>>>>>>> changes. I
>>>>>>>> also split out the ARMAsmPrinter fix into it's own patch.
>>>>>>>>
>>>>>>>> deep
>>>>>>>>
>>>>>>>> On Mon, Feb 9, 2009 at 8:54 AM, Evan Cheng <echeng at apple.com>
>>>>>>>> wrote:
>>>>>>>>> Thanks Sandeep. I did a quick scan, this looks really good.
>>>>>>>>> But I
>>>>>>>>> do
>>>>>>>>> have a question:
>>>>>>>>>
>>>>>>>>> +/// CCCustomFn - This function assigns a location for Val,
>>>>>>>>> possibly
>>>>>>>>> updating
>>>>>>>>> +/// all args to reflect changes and indicates if it handled
>>>>>>>>> it. It
>>>>>>>>> must set
>>>>>>>>> +/// isCustom if it handles the arg and returns true.
>>>>>>>>> +typedef bool CCCustomFn(unsigned &ValNo, MVT &ValVT,
>>>>>>>>> + MVT &LocVT, CCValAssign::LocInfo
>>>>>>>>> &LocInfo,
>>>>>>>>> + ISD::ArgFlagsTy &ArgFlags, CCState
>>>>>>>>> &State,
>>>>>>>>> + bool &result);
>>>>>>>>>
>>>>>>>>> Is it necessary to return two bools (the second is returned by
>>>>>>>>> reference in 'result')? I am confused about the semantics of
>>>>>>>>> 'result'.
>>>>>>>>>
>>>>>>>>> Also, a nitpick:
>>>>>>>>>
>>>>>>>>> + unsigned i;
>>>>>>>>> + for (i = 0; i < 4; ++i)
>>>>>>>>>
>>>>>>>>> The convention we use is:
>>>>>>>>>
>>>>>>>>> + for (unsigned i = 0; i < 4; ++i)
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Evan
>>>>>>>>>
>>>>>>>>> On Feb 6, 2009, at 6:02 PM, Sandeep Patel wrote:
>>>>>>>>>
>>>>>>>>>> I think I've got all the cases handled now, implementing with
>>>>>>>>>> CCCustom<"foo"> callbacks into C++.
>>>>>>>>>>
>>>>>>>>>> This also fixes a crash when returning i128. I've also
>>>>>>>>>> included a
>>>>>>>>>> small asm constraint fix that was needed to build newlib.
>>>>>>>>>>
>>>>>>>>>> deep
>>>>>>>>>>
>>>>>>>>>> On Mon, Jan 19, 2009 at 10:18 AM, Evan Cheng
>>>>>>>>>> <evan.cheng at apple.com>
>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>> On Jan 16, 2009, at 5:26 PM, Sandeep Patel wrote:
>>>>>>>>>>>
>>>>>>>>>>>> On Sat, Jan 3, 2009 at 11:46 AM, Dan Gohman <gohman at apple.com
>>>>>>>>>>>> >
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> One problem with this approach is that since i64 isn't
>>>>>>>>>>>>> legal,
>>>>>>>>>>>>> the
>>>>>>>>>>>>> bitcast would require custom C++ code in the ARM target to
>>>>>>>>>>>>> handle properly. It might make sense to introduce
>>>>>>>>>>>>> something
>>>>>>>>>>>>> like
>>>>>>>>>>>>>
>>>>>>>>>>>>> CCIfType<[f64], CCCustom>
>>>>>>>>>>>>>
>>>>>>>>>>>>> where CCCustom is a new entity that tells the calling
>>>>>>>>>>>>> convention
>>>>>>>>>>>>> code to to let the target do something not easily
>>>>>>>>>>>>> representable
>>>>>>>>>>>>> in the tablegen minilanguage.
>>>>>>>>>>>>
>>>>>>>>>>>> I am thinking that this requires two changes: add a flag to
>>>>>>>>>>>> CCValAssign (take a bit from HTP) to indicate isCustom
>>>>>>>>>>>> and a
>>>>>>>>>>>> way
>>>>>>>>>>>> to
>>>>>>>>>>>> author an arbitrary CCAction by including the source
>>>>>>>>>>>> directly in
>>>>>>>>>>>> the
>>>>>>>>>>>> TableGen mini-language. This latter change might want a
>>>>>>>>>>>> generic
>>>>>>>>>>>> change
>>>>>>>>>>>> to the TableGen language. For example, the syntax might be
>>>>>>>>>>>> like:
>>>>>>>>>>>>
>>>>>>>>>>>> class foo : CCCustomAction {
>>>>>>>>>>>> code <<< EOF
>>>>>>>>>>>> ....multi-line C++ code goes here that allocates regs & mem
>>>>>>>>>>>> and
>>>>>>>>>>>> sets CCValAssign::isCustom....
>>>>>>>>>>>> EOF
>>>>>>>>>>>> }
>>>>>>>>>>>>
>>>>>>>>>>>> Does this seem reasonable? An alternative is for CCCustom
>>>>>>>>>>>> to
>>>>>>>>>>>> take a
>>>>>>>>>>>> string that names a function to be called:
>>>>>>>>>>>>
>>>>>>>>>>>> CCIfType<[f64], CCCustom<"MyCustomLoweringFunc">>
>>>>>>>>>>>>
>>>>>>>>>>>> the function signature for such functions will have to
>>>>>>>>>>>> return
>>>>>>>>>>>> two
>>>>>>>>>>>> results: if the CC processing is finished and if it the
>>>>>>>>>>>> func
>>>>>>>>>>>> succeeded
>>>>>>>>>>>> or failed:
>>>>>>>>>>>
>>>>>>>>>>> I like the second solution better. It seems rather
>>>>>>>>>>> cumbersome
>>>>>>>>>>> to
>>>>>>>>>>> embed
>>>>>>>>>>> multi-line c++ code in td files.
>>>>>>>>>>>
>>>>>>>>>>> Evan
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> typedef bool CCCustomFn(unsigned ValNo, MVT ValVT,
>>>>>>>>>>>> MVT LocVT, CCValAssign::LocInfo LocInfo,
>>>>>>>>>>>> ISD::ArgFlagsTy ArgFlags, CCState &State,
>>>>>>>>>>>> bool &result);
>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>> LLVM Developers mailing list
>>>>>>>>>>>> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
>>>>>>>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>>>>>>>>>>
>>>>>>>>>>> _______________________________________________
>>>>>>>>>>> LLVM Developers mailing list
>>>>>>>>>>> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
>>>>>>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>>>>>>>>>>
>>>>>>>>>> <
>>>>>>>>>> arm_callingconv
>>>>>>>>>> .diff>_______________________________________________
>>>>>>>>>> LLVM Developers mailing list
>>>>>>>>>> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
>>>>>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>>>>>>>>
>>>>>>>>> _______________________________________________
>>>>>>>>> LLVM Developers mailing list
>>>>>>>>> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
>>>>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>>>>>>>>
>>>>>>>> <
>>>>>>>> arm_callingconv
>>>>>>>> .diff
>>>>>>>>> <
>>>>>>>>> arm_fixes.diff>_______________________________________________
>>>>>>>> LLVM Developers mailing list
>>>>>>>> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
>>>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> LLVM Developers mailing list
>>>>>>> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
>>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>>>>>>
>>>>>> _______________________________________________
>>>>>> LLVM Developers mailing list
>>>>>> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>>>>
>>>>> _______________________________________________
>>>>> LLVM Developers mailing list
>>>>> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>>>>
>>>> _______________________________________________
>>>> LLVM Developers mailing list
>>>> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>>
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>>
>>
> <
> arm_callingconv
> .diff><arm_fixes.diff>_______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
More information about the llvm-dev
mailing list