> -----Original Message----- > From: Duncan Sands [mailto:baldrick at free.fr] > Sent: Friday, January 09, 2009 5:23 PM > To: Sanjiv Kumar Gupta - I00171 > Cc: llvmdev at cs.uiuc.edu > Subject: Re: PIC16 backend for llvm 2.5 > > Hi Sanjiv, > > > Well, the first email is here. > > > > http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon- > 20081013/068667.html > > thanks, I remember now (more or less). So would something like > ReplaceNodeResults > solve the problem? >Well the magnitude of the task is not small. ExpandIntegerOperand() calls LowerOperation() to allow targets to handle illegal operands. So we will need to change the interface of LowerOperation() to pass an extra argument called Results, which is an array of SDValue. Targets will push the result values in this array and then we can replace values in ExpandIntegerOperand(). Very much like what CustomLowerResults() and ReplaceNodeResults() are doing currently. The problem is that do we want to change calls to LowerOperation() in LegalizeDAG as well? I think probably that is the right approach to go in the longer term. But currently I suggest that "Results" be the last argument to LowerOperation() which is defaulted to NULL. That way LegalizeDAG and all targets will continue to work the current way, plus targets like ours that want to use the last argument (i.e. "Results") can use them in ExapndIntegerOperand(). Let me know if that sounds okay. Regards, Sanjiv> Ciao, > > Duncan.
Hi Sanjiv,> Well the magnitude of the task is not small. > ExpandIntegerOperand() calls LowerOperation() to allow targets to handle > illegal operands. So we will need to change the interface of > LowerOperation() to pass an extra argument called Results, which is an > array of SDValue. Targets will push the result values in this array and > then we can replace values in ExpandIntegerOperand(). Very much like > what CustomLowerResults() and ReplaceNodeResults() are doing currently. > > The problem is that do we want to change calls to LowerOperation() in > LegalizeDAG as well? I think probably that is the right approach to go > in the longer term. But currently I suggest that "Results" be the last > argument to LowerOperation() which is defaulted to NULL. That way > LegalizeDAG and all targets will continue to work the current way, plus > targets like ours that want to use the last argument (i.e. "Results") > can use them in ExapndIntegerOperand().do you need this for operation legalization (LegalizeDAG) as well as type legalization? If not, then you can introduce a new method like ReplaceNodeResults for custom type legalization of operands (or just use ReplaceNodeResults for this too - I don't immediately see any reason why not), and have it call LowerOperation by default. Actually, if you also want this for LegalizeDAG too, you can introduce a new method which is only called in places that need it; everywhere else can still use LowerOperation (of course in the long term there should be just one method, but this way you can do things incrementally). I don't much like the idea of having Results be NULL. I'd rather the interface was uniform, and have any tricks be in the body of the method. Ciao, Duncan.
On Fri, 2009-01-16 at 10:03 +0100, Duncan Sands wrote:> Hi Sanjiv, > > > Well the magnitude of the task is not small. > > ExpandIntegerOperand() calls LowerOperation() to allow targets to handle > > illegal operands. So we will need to change the interface of > > LowerOperation() to pass an extra argument called Results, which is an > > array of SDValue. Targets will push the result values in this array and > > then we can replace values in ExpandIntegerOperand(). Very much like > > what CustomLowerResults() and ReplaceNodeResults() are doing currently. > > > > The problem is that do we want to change calls to LowerOperation() in > > LegalizeDAG as well? I think probably that is the right approach to go > > in the longer term. But currently I suggest that "Results" be the last > > argument to LowerOperation() which is defaulted to NULL. That way > > LegalizeDAG and all targets will continue to work the current way, plus > > targets like ours that want to use the last argument (i.e. "Results") > > can use them in ExapndIntegerOperand(). > > do you need this for operation legalization (LegalizeDAG) as well as > type legalization? If not, then you can introduce a new method like > ReplaceNodeResults for custom type legalization of operands (or just > use ReplaceNodeResults for this too - I don't immediately see any > reason why not), and have it call LowerOperation by default. Actually, > if you also want this for LegalizeDAG too, you can introduce a new method > which is only called in places that need it; everywhere else can still > use LowerOperation (of course in the long term there should be just one > method, but this way you can do things incrementally). I don't much > like the idea of having Results be NULL. I'd rather the interface was > uniform, and have any tricks be in the body of the method. > > Ciao, > > Duncan.Thanks Duncan for your suggestions. We have worked out a patch accordingly. The patch is attached, let me know if it looks okay to commit. Thanks, Sanjiv -------------- next part -------------- A non-text attachment was scrubbed... Name: patch.txt Type: text/x-patch Size: 10530 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20090119/ecff7e12/attachment.bin>