[LLVMdev] [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/

[Scott Michel]

Chris:

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:

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.
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.
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.)

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.

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.

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. :-)

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.


-scooter

On Mon, Feb 23, 2009 at 12:49 PM, Chris Lattner <clattner at apple.com> wrote:

> fyi in case you're not reading commits.
>
> Begin forwarded message:
>
>  From: Chris Lattner <clattner at apple.com>
>> Date: February 23, 2009 12:48:33 PM PST
>> To: Commit Messages and Patches for LLVM <llvm-commits at cs.uiuc.edu>
>> 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/
>> Reply-To: Commit Messages and Patches for LLVM <llvm-commits at cs.uiuc.edu>
>>
>>
>> On Feb 22, 2009, at 3:36 PM, Scott Michel wrote:
>>
>>  Author: pingbak
>>> Date: Sun Feb 22 17:36:09 2009
>>> New Revision: 65296
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=65296&view=rev
>>> Log:
>>> Introduce the BuildVectorSDNode class that encapsulates the
>>> ISD::BUILD_VECTOR
>>> instruction. The class also consolidates the code for detecting
>>> constant
>>> splats that's shared across PowerPC and the CellSPU backends (and
>>> might be
>>> useful for other backends.) Also introduces
>>> SelectionDAG::getBUID_VECTOR() for
>>> generating new BUILD_VECTOR nodes.
>>>
>>
>> Hi Scott,
>>
>> Before making a major change like this it really is best to float the
>> idea by the list.
>>
>> I don't see any reason to make this sort of change.  Sharing code
>> between targets is definitely goodness, but the way you are doing this
>> seems very strange.  Instead of creating a new SDNode class, why not
>> move the various functionality to helper functions on SDNode or
>> SelectionDAG?
>>
>> -Chris
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20090223/5b89fcc6/attachment.html>