Dan Gohman
2012-Apr-19 23:27 UTC
[LLVMdev] [llvm-commits] [PATCH][RFC] Add extra arguments to TargetLowering::LowerCall() so targets have more context in which to construct call chains
On Apr 19, 2012, at 12:46 PM, Evan Cheng <evan.cheng at apple.com> wrote:> > On Apr 19, 2012, at 11:15 AM, Justin Holewinski wrote: > >> >> From: Evan Cheng [mailto:evan.cheng at apple.com] >> Sent: Thursday, April 19, 2012 10:47 AM >> To: Justin Holewinski >> Cc: llvmdev at cs.uiuc.edu; llvm-commits at cs.uiuc.edu; Vinod Grover >> Subject: Re: [llvm-commits] [PATCH][RFC] Add extra arguments to TargetLowering::LowerCall() so targets have more context in which to construct call chains >> >> TargetLowering::LowerCall is already a mess, I would really prefer not to extend it any further. It's especially difficult to justify extending it without a use in the open source tree. >> >> We really should think hard about how to improve the API in two ways. Perhaps we should wrap the arguments in some struct rather than as individual ones. We should also make it easier to extend it in the future without requiring an across the change for all in-tree and out-of-tree targets. Anyone wants to take the lead on this? >> >> The second may be solved by the first. If the LowerCall() arguments are encapsulated within some descriptor object, then extensions to this descriptor class would be transparent to existing targets, both in-tree and out-of-tree (assuming the API is consistent). What is the overall plan for LowerCall() and LowerCallTo()? I noticed the comment on LowerCallTo() that implies it should be merged into SelectionDAGISel when all targets are using LowerCall (which I believe they are)? > > I believe Dan has some thoughts in this topic.That FIXME comment about LowerCallTo is probably an anachronism, and no longer relevant given the changes that that code has been through. LowerCallTo is currently a wrapper around LowerCall which various things need. All targets which support calls are using LowerCall. Dan
Justin Holewinski
2012-May-15 13:56 UTC
[LLVMdev] [llvm-commits] [PATCH][RFC] Add extra arguments to TargetLowering::LowerCall() so targets have more context in which to construct call chains
In response to this discussion, I've prepared the attached patch as an initial prototype for the LowerCall/LowerCallTo change. All of the information currently needed by the back-ends, and the extra information needed by the NVPTX back-end, is now wrapped in a CallLoweringInfo struct. The various SelectionDAG classes have been modified so any calls to TargetLowering::LowerCallTo use this struct, and this struct is the means for passing information to/from each target's LowerCall method. Right now, the interface is a bit rough around the edges, but I wanted to get feedback from the community on what you want in this interface before spending too much time on it. This patch currently only addresses the X86 and NVPTX back-ends. Once the interface is solidified, I will update the rest of the in-tree targets.> -----Original Message----- > From: Dan Gohman [mailto:gohman at apple.com] > Sent: Thursday, April 19, 2012 4:28 PM > To: Evan Cheng > Cc: Justin Holewinski; llvmdev at cs.uiuc.edu; Commit Messages and Patches > for LLVM; Vinod Grover > Subject: Re: [llvm-commits] [PATCH][RFC] Add extra arguments to > TargetLowering::LowerCall() so targets have more context in which to > construct call chains > > > On Apr 19, 2012, at 12:46 PM, Evan Cheng <evan.cheng at apple.com> wrote: > > > > > On Apr 19, 2012, at 11:15 AM, Justin Holewinski wrote: > > > >> > >> From: Evan Cheng [mailto:evan.cheng at apple.com] > >> Sent: Thursday, April 19, 2012 10:47 AM > >> To: Justin Holewinski > >> Cc: llvmdev at cs.uiuc.edu; llvm-commits at cs.uiuc.edu; Vinod Grover > >> Subject: Re: [llvm-commits] [PATCH][RFC] Add extra arguments to > TargetLowering::LowerCall() so targets have more context in which to > construct call chains > >> > >> TargetLowering::LowerCall is already a mess, I would really prefer not to > extend it any further. It's especially difficult to justify extending it without a > use in the open source tree. > >> > >> We really should think hard about how to improve the API in two ways. > Perhaps we should wrap the arguments in some struct rather than as > individual ones. We should also make it easier to extend it in the future > without requiring an across the change for all in-tree and out-of-tree targets. > Anyone wants to take the lead on this? > >> > >> The second may be solved by the first. If the LowerCall() arguments are > encapsulated within some descriptor object, then extensions to this > descriptor class would be transparent to existing targets, both in-tree and > out-of-tree (assuming the API is consistent). What is the overall plan for > LowerCall() and LowerCallTo()? I noticed the comment on LowerCallTo() that > implies it should be merged into SelectionDAGISel when all targets are using > LowerCall (which I believe they are)? > > > > I believe Dan has some thoughts in this topic. > > That FIXME comment about LowerCallTo is probably an anachronism, > and no longer relevant given the changes that that code has been > through. LowerCallTo is currently a wrapper around LowerCall > which various things need. All targets which support calls are > using LowerCall. > > Dan----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. ----------------------------------------------------------------------------------- -------------- next part -------------- A non-text attachment was scrubbed... Name: llvm-lowercallto-refactor.patch Type: application/octet-stream Size: 34448 bytes Desc: llvm-lowercallto-refactor.patch URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20120515/5f232286/attachment.obj>
Evan Cheng
2012-May-17 00:30 UTC
[LLVMdev] [llvm-commits] [PATCH][RFC] Add extra arguments to TargetLowering::LowerCall() so targets have more context in which to construct call chains
Thanks. This is going in the right direction IMHO. Obviously, please make sure you add comments to the data structure and convert all the targets first. Also, please don't forget to send an email to llvmdev to warn owners of all the out-of-tree targets about the ABI change. Evan On May 15, 2012, at 6:56 AM, Justin Holewinski <jholewinski at nvidia.com> wrote:> In response to this discussion, I've prepared the attached patch as an initial prototype for the LowerCall/LowerCallTo change. All of the information currently needed by the back-ends, and the extra information needed by the NVPTX back-end, is now wrapped in a CallLoweringInfo struct. The various SelectionDAG classes have been modified so any calls to TargetLowering::LowerCallTo use this struct, and this struct is the means for passing information to/from each target's LowerCall method. Right now, the interface is a bit rough around the edges, but I wanted to get feedback from the community on what you want in this interface before spending too much time on it. This patch currently only addresses the X86 and NVPTX back-ends. Once the interface is solidified, I will update the rest of the in-tree targets. > >> -----Original Message----- >> From: Dan Gohman [mailto:gohman at apple.com] >> Sent: Thursday, April 19, 2012 4:28 PM >> To: Evan Cheng >> Cc: Justin Holewinski; llvmdev at cs.uiuc.edu; Commit Messages and Patches >> for LLVM; Vinod Grover >> Subject: Re: [llvm-commits] [PATCH][RFC] Add extra arguments to >> TargetLowering::LowerCall() so targets have more context in which to >> construct call chains >> >> >> On Apr 19, 2012, at 12:46 PM, Evan Cheng <evan.cheng at apple.com> wrote: >> >>> >>> On Apr 19, 2012, at 11:15 AM, Justin Holewinski wrote: >>> >>>> >>>> From: Evan Cheng [mailto:evan.cheng at apple.com] >>>> Sent: Thursday, April 19, 2012 10:47 AM >>>> To: Justin Holewinski >>>> Cc: llvmdev at cs.uiuc.edu; llvm-commits at cs.uiuc.edu; Vinod Grover >>>> Subject: Re: [llvm-commits] [PATCH][RFC] Add extra arguments to >> TargetLowering::LowerCall() so targets have more context in which to >> construct call chains >>>> >>>> TargetLowering::LowerCall is already a mess, I would really prefer not to >> extend it any further. It's especially difficult to justify extending it without a >> use in the open source tree. >>>> >>>> We really should think hard about how to improve the API in two ways. >> Perhaps we should wrap the arguments in some struct rather than as >> individual ones. We should also make it easier to extend it in the future >> without requiring an across the change for all in-tree and out-of-tree targets. >> Anyone wants to take the lead on this? >>>> >>>> The second may be solved by the first. If the LowerCall() arguments are >> encapsulated within some descriptor object, then extensions to this >> descriptor class would be transparent to existing targets, both in-tree and >> out-of-tree (assuming the API is consistent). What is the overall plan for >> LowerCall() and LowerCallTo()? I noticed the comment on LowerCallTo() that >> implies it should be merged into SelectionDAGISel when all targets are using >> LowerCall (which I believe they are)? >>> >>> I believe Dan has some thoughts in this topic. >> >> That FIXME comment about LowerCallTo is probably an anachronism, >> and no longer relevant given the changes that that code has been >> through. LowerCallTo is currently a wrapper around LowerCall >> which various things need. All targets which support calls are >> using LowerCall. >> >> Dan > > > ----------------------------------------------------------------------------------- > This email message is for the sole use of the intended recipient(s) and may contain > confidential information. Any unauthorized review, use, disclosure or distribution > is prohibited. If you are not the intended recipient, please contact the sender by > reply email and destroy all copies of the original message. > ----------------------------------------------------------------------------------- > <llvm-lowercallto-refactor.patch>_______________________________________________ > llvm-commits mailing list > llvm-commits at cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
Reasonably Related Threads
- [LLVMdev] [llvm-commits] [PATCH][RFC] Add extra arguments to TargetLowering::LowerCall() so targets have more context in which to construct call chains
- [LLVMdev] [llvm-commits] [PATCH][RFC] Add extra arguments to TargetLowering::LowerCall() so targets have more context in which to construct call chains
- [LLVMdev] [llvm-commits] [PATCH][RFC] Add extra arguments to TargetLowering::LowerCall() so targets have more context in which to construct call chains
- [LLVMdev] [llvm-commits] [PATCH][RFC] Add extra arguments to TargetLowering::LowerCall() so targets have more context in which to construct call chains
- [LLVMdev] [llvm-commits] [PATCH][RFC] Add extra arguments to TargetLowering::LowerCall() so targets have more context in which to construct call chains