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. 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 >
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
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 >-------------- next part -------------- A non-text attachment was scrubbed... Name: arm_callingconv.diff Type: application/octet-stream Size: 54711 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20090213/8cc9cfc6/attachment.obj> -------------- next part -------------- A non-text attachment was scrubbed... Name: arm_fixes.diff Type: application/octet-stream Size: 590 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20090213/8cc9cfc6/attachment-0001.obj>