[patch] DAG nodes IR ordering code re-factoring

[Andrew Trick]

On May 24, 2013, at 5:47 PM, "Guo, Xiaoyi" <Xiaoyi.Guo at amd.com> wrote:

> Hi Andy,
>  
> Thanks for doing the memory impact analysis. Attached please find the new set of patches, with all review comments addressed. There’s one exception. The diff can only remove the content of SDNodeOrdering.h, but not the file. You may have to manually remove it using svn command.
>  
> [Patch 1/4]: Add new fields and utility classes without changing existing interfaces or functionalities.
> [Patch 2/4]: Change SelectionDAG::getXXXNode() interfaces as well as call sites of these functions to pass in SDLoc instead of DebugLoc.
> [Patch 3/4]: Remove old IR ordering mechanism and switch to new one. Fix unit test failures.
> [Patch 4/4]: Add unit tests.

Committed: r182701,r182703,r182704,r182706

Thanks! I just realized I forgot to properly attribute the commit log. I’ll send a message to llvmdev tomorrow to make up for it.
-Andy

>  
> Thanks,
> Xiaoyi
>  
> From: Andrew Trick [mailto:atrick at apple.com] 
> Sent: Thursday, May 23, 2013 3:41 PM
> To: Guo, Xiaoyi
> Cc: Tom Stellard; llvm commits; Stellard, Thomas; Chris Lattner
> Subject: Re: [patch] DAG nodes IR ordering code re-factoring
>  
> On May 21, 2013, at 3:58 PM, Chris Lattner <clattner at apple.com> wrote:
> On May 21, 2013, at 11:43 AM, "Guo, Xiaoyi" <Xiaoyi.Guo at amd.com> wrote:
> 
> 
> Hi Tom,
> 
> Here's the email thread where I explained my motivation and responses from other people.
> http://lists.cs.uiuc.edu/pipermail/llvmdev/2013-April/061728.html
> 
> I'm not thrilled about making SDNode larger.  Have you quantified the memory impact of this change?
>  
> The IROrder field fits in padding just before debugLoc. I verified that the bumpPtrAllocator stats don't change when the field is placed optimally.
>  
> Incidentally, we already allocate 256 bytes for every SDNode! That's huge compared to IR values and MI instructions. Most of that space is unused for common SDNode types.
>  
> If we later want to squeeze more out of the SDNode bits, we can combine IROrder with nodeid which are essentially redundant. Reusing nodeid will take some work though--it's something I'll consider when designing the linearizer to bypass the SD scheduler.
>  
> More importantly, moving IROrder to a field actually reduces memory, in addition to being much more efficient. Xaioyi's second patch tracks IR location for most nodes. This tracking will be done all the time (except fast isel)--it's not really optional. We don't want to create hash table entries for most of the SDNodes. Xiaoyi's third patch deletes that giant hash table. On 403.gcc that reduces the memory footprint of SelectBasicBlock by 2MB out of 38MB total. That's pretty good!
>  
> Xiaoyi: In your final patch set please do the following
> - move the IROrder field to just below debugLoc
> - make sure patch #3 deletes SDNodeOrdering.h
> - initialize CurInst=0
>  
> I don't have any more suggestions. If you've addressed all the review comments, please send a final patch set and I'll check it in. These patches allow pre-RA-sched=source to work as intended. Enabling pre-RA-sched=source is a side effect of enabling the MachineScheduler, so it would be nice to have it working correctly when I do that.
>  
> -Andy
> <Mail Attachment><Mail Attachment><Mail Attachment><Mail Attachment>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130524/bc49239d/attachment.html>