Chris:<br><br>I did float this by the dev list first a couple of weeks ago, didn't receive any comments. It's not entirely gratuitous; the rationale for adding a new node class is threefold:<br><br>a) Convenience for the backends. Since it benefits multiple backends (PPC and CellSPU), it's a logical addition. I reckon the GPU efforts would also benefit.<br>
b) Where else would one encapsulate a constant splat predicate? SelectionDAG and SDNode are not good classes for constant splat detection, since it's functionality specific to building vectors.<br>c) Future work. At some point (or another), having target-specific SDNode polymorphism is an issue that has to be addressed, in light of the vector swizzling support conversation thread. One wants the benefits of what the legalization and DAG combiner phases provide, but not have to add more code for a target-specific ISD enumeration value with a generic SDNode, if that functionality is already available (and doesn't need to be overridden.)<br>
<br>It seems to me that LLVM will eventually need to have a bijective mapping between the ISD enumeration and the SDNodes, in the case of (c). (c) is a huge undertaking and needs careful thought. For example, the whole "Is the node legal, custom, promotable or expanded" testing screams for static protocol methods hooked to per-target protected virtual methods.<br>
<br>But (a) and (b) are the dominant factors for contributing the change. It's not completely altruistic: the commit makes it considerably easier for the CellSPU backend to deal with various splats at instruction selection rather than during target-dependent lowering.<br>
<br>I attempted to be pretty thorough with the change before committing. I ran through the tests before committing. The changes were primarily method invocations, which admittedly were pretty numerous. It wasn't as if I was adding DebugLoc and changing prototypes everywhere. :-)<br>
<br>Ok, I'm sure there were places in clang and llvm-gcc-4.2 that I overlooked, and I appologize for being narrowly focused on the backend infrastructure.<br><br><br>-scooter<br><br><div class="gmail_quote">On Mon, Feb 23, 2009 at 12:49 PM, Chris Lattner <span dir="ltr"><<a href="mailto:clattner@apple.com">clattner@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">fyi in case you're not reading commits.<br>
<br>
Begin forwarded message:<br>
<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
From: Chris Lattner <<a href="mailto:clattner@apple.com" target="_blank">clattner@apple.com</a>><br>
Date: February 23, 2009 12:48:33 PM PST<br>
To: Commit Messages and Patches for LLVM <<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a>><br>
Subject: Re: [llvm-commits] [llvm] r65296 - in /llvm/trunk: include/llvm/CodeGen/ lib/CodeGen/SelectionDAG/ lib/Target/CellSPU/ lib/Target/PowerPC/ lib/Target/X86/ test/CodeGen/X86/<br>
Reply-To: Commit Messages and Patches for LLVM <<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a>><br>
<br>
<br>
On Feb 22, 2009, at 3:36 PM, Scott Michel wrote:<br>
<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Author: pingbak<br>
Date: Sun Feb 22 17:36:09 2009<br>
New Revision: 65296<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=65296&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=65296&view=rev</a><br>
Log:<br>
Introduce the BuildVectorSDNode class that encapsulates the<br>
ISD::BUILD_VECTOR<br>
instruction. The class also consolidates the code for detecting<br>
constant<br>
splats that's shared across PowerPC and the CellSPU backends (and<br>
might be<br>
useful for other backends.) Also introduces<br>
SelectionDAG::getBUID_VECTOR() for<br>
generating new BUILD_VECTOR nodes.<br>
</blockquote>
<br>
Hi Scott,<br>
<br>
Before making a major change like this it really is best to float the<br>
idea by the list.<br>
<br>
I don't see any reason to make this sort of change.  Sharing code<br>
between targets is definitely goodness, but the way you are doing this<br>
seems very strange.  Instead of creating a new SDNode class, why not<br>
move the various functionality to helper functions on SDNode or<br>
SelectionDAG?<br>
<br>
-Chris<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote>
<br>
</blockquote></div><br>