llvm dev - Feb 2009 - [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/

> 3. Introduce a new ShuffleVectorSDNode that only has two SDValue  
> operands (the two input vectors), but that also contains an array of  
> ints in the node (not as operands).
...
> The important part of #3 is that we really want an array of ints  
> (using -1 for undef) for the shuffle mask, not "operands".  This
> eliminates the nastiness we have now were we need a buildvector, and  
> it eliminates the dance we have to prevent the build vector from being  
> legalized, and prevent the integer operands to it from being legalized.
This is PR2957 (which originally suggested a variadic SDNode, but it
quickly became clear that an array of ints is better).  It would be
great to have a volunteer for this (I don't have time).

Ciao,

Duncan.

Duncan:

I'm still stymied how this whole thread ended up about shuffle vector nodes,
when the original problem was my build vector patch. I'm still working on
backing the build vector patch out (it isn't clean with all of the
intervening commits and I have pressing management tasks which command my
attention.)


-scooter

On Tue, Feb 24, 2009 at 12:28 AM, Duncan Sands <baldrick at free.fr>
wrote:
> > 3. Introduce a new ShuffleVectorSDNode that only has two SDValue
> > operands (the two input vectors), but that also contains an array of
> > ints in the node (not as operands).
> ...
> > The important part of #3 is that we really want an array of ints
> > (using -1 for undef) for the shuffle mask, not "operands". 
This
> > eliminates the nastiness we have now were we need a buildvector, and
> > it eliminates the dance we have to prevent the build vector from being
> > legalized, and prevent the integer operands to it from being
legalized.
>
> This is PR2957 (which originally suggested a variadic SDNode, but it
> quickly became clear that an array of ints is better).  It would be
> great to have a volunteer for this (I don't have time).
>
> Ciao,
>
> Duncan.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20090224/bc2ae490/attachment.html>

I believe this patch has broken a PPC app that I am tracking. Here is  
a reduced test case. Reproduce with llc -mattr=+Altivec -mcpu=g5. The  
backtrace looks like this:

#0  0x9333ae42 in __kill ()
#1  0x9333ae34 in kill$UNIX2003 ()
#2  0x933ad23a in raise ()
#3  0x933b9679 in abort ()
#4  0x933ae3db in __assert_rtn ()
#5  0x0008bd8f in llvm::MVT::getVectorElementType (this=0xbfffdda4) at  
ValueTypes.h:317
#6  0x002aed06 in BuildSplatI (Val=0, SplatSize=8, VT={{V = 24,  
SimpleTy = llvm::MVT::v4i32, LLVMTy = 0x18}}, DAG=@0x16088a0, dl={Idx  
= 4294967295}) at PPCISelLowering.cpp:311\
5
#7  0x002afae4 in llvm::PPCTargetLowering::LowerBUILD_VECTOR  
(this=0x1803d58, Op={Node = 0x157a530, ResNo = 0}, DAG=@0x16088a0) at  
PPCISelLowering.cpp:3200
#8  0x002bb54f in llvm::PPCTargetLowering::LowerOperation  
(this=0x1803d58, Op={Node = 0x157a530, ResNo = 0}, DAG=@0x16088a0) at  
PPCISelLowering.cpp:3766
#9  0x0051bed6 in (anonymous  
namespace)::SelectionDAGLegalize::LegalizeOp (this=0xbffff0e8,  
Op={Node = 0x157a530, ResNo = 0}) at LegalizeDAG.cpp:1608
#10 0x0054837d in (anonymous  
namespace)::SelectionDAGLegalize::HandleOp (this=0xbffff0e8, Op={Node  
= 0x157a530, ResNo = 0}) at LegalizeDAG.cpp:519
#11 0x005485a5 in (anonymous  
namespace)::SelectionDAGLegalize::LegalizeDAG (this=0xbffff0e8) at  
LegalizeDAG.cpp:389
#12 0x00548734 in llvm::SelectionDAG::Legalize (this=0x16088a0,  
TypesNeedLegalizing=false, Fast=false) at LegalizeDAG.cpp:8648
#13 0x005ec313 in llvm::SelectionDAGISel::CodeGenAndEmitDAG  
(this=0x1608780) at SelectionDAGISel.cpp:626
#14 0x005ee7e2 in llvm::SelectionDAGISel::SelectBasicBlock  
(this=0x1608780, LLVMBB=0x1603fa0,  
Begin={<bidirectional_iterator<llvm::Instruction,int>> =  
{<std::iterator<std::bid\
irectional_iterator_tag 
,llvm::Instruction,int,llvm::Instruction*,llvm::Instruction&>> =
{<No
data fields>}, <No data fields>}, NodePtr = 0x1604dd0},  
End={<bidirectional_iterat\
or<llvm::Instruction,int>> =  
{< 
std 
::iterator 
< 
std 
::bidirectional_iterator_tag 
,llvm::Instruction,int,llvm::Instruction*,llvm::Instruction&>> =
{<No
data fields>}, <No data field\
s>}, NodePtr = 0x16049e0}) at SelectionDAGISel.cpp:500
#15 0x005ef123 in llvm::SelectionDAGISel::SelectAllBasicBlocks  
(this=0x1608780, Fn=@0x1603720, MF=@0x160d520, MMI=0x160bbd0,  
DW=0x1608fe0, TII=@0x1803ce0) at SelectionDAGISel.\
cpp:856
#16 0x005efe54 in llvm::SelectionDAGISel::runOnFunction  
(this=0x1608780, Fn=@0x1603720) at SelectionDAGISel.cpp:327
#17 0x002a3aea in (anonymous  
namespace)::PPCDAGToDAGISel::runOnFunction (this=0x1608780,  
Fn=@0x1603720) at PPCISelDAGToDAG.cpp:54
#18 0x00874127 in llvm::FPPassManager::runOnFunction (this=0x1606610,  
F=@0x1603720) at PassManager.cpp:1323
#19 0x0087464c in llvm::FunctionPassManagerImpl::run (this=0x1606410,  
F=@0x1603720) at PassManager.cpp:1281
#20 0x008747da in llvm::FunctionPassManager::run (this=0xbffff520,  
F=@0x1603720) at PassManager.cpp:1226
#21 0x0000352e in main (argc=6, argv=0xbffff5d0) at llc.cpp:317



Evan

On Feb 24, 2009, at 9:35 AM, Scott Michel wrote:
> Duncan:
>
> I'm still stymied how this whole thread ended up about shuffle  
> vector nodes, when the original problem was my build vector patch.  
> I'm still working on backing the build vector patch out (it isn't  
> clean with all of the intervening commits and I have pressing  
> management tasks which command my attention.)
>
>
> -scooter
>
> On Tue, Feb 24, 2009 at 12:28 AM, Duncan Sands <baldrick at free.fr>
> wrote:
> > 3. Introduce a new ShuffleVectorSDNode that only has two SDValue
> > operands (the two input vectors), but that also contains an array of
> > ints in the node (not as operands).
> ...
> > The important part of #3 is that we really want an array of ints
> > (using -1 for undef) for the shuffle mask, not "operands". 
This
> > eliminates the nastiness we have now were we need a buildvector, and
> > it eliminates the dance we have to prevent the build vector from  
> being
> > legalized, and prevent the integer operands to it from being  
> legalized.
>
> This is PR2957 (which originally suggested a variadic SDNode, but it
> quickly became clear that an array of ints is better).  It would be
> great to have a volunteer for this (I don't have time).
>
> Ciao,
>
> Duncan.
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20090224/7e9971a4/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bugpoint-reduced-simplified.bc
Type: application/octet-stream
Size: 1568 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20090224/7e9971a4/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20090224/7e9971a4/attachment-0001.html>

On Feb 24, 2009, at 9:35 AM, Scott Michel wrote:
> Duncan:
>
> I'm still stymied how this whole thread ended up about shuffle  
> vector nodes, when the original problem was my build vector patch.  
> I'm still working on backing the build vector patch out (it isn't  
> clean with all of the intervening commits and I have pressing  
> management tasks which command my attention.)
Scott,

You're absolutely right.  I got very very confused early on and that  
completely derailed the whole discussion.  I'm sorry for being an  
idiot, I must have had crossed wires.

Now that I actually understand that your goal here is to handle  
*buildvectors* not "the buildvector argument to a shuffle node", I  
understand a lot better what you're trying to do (and agree that it  
has nothing to do with shuffles!).

However, I still prefer a different approach.  Are you familiar with  
the IntrinsicInst class and things like DbgStopPointInst?  The basic  
jist of it is that they are "pseudo classes" that are used with  
dyn_cast etc that provide very useful helpers.

In the case of LLVM IR and debug stoppoint, a stoppoint is just an  
llvm call instruction which happens to call the llvm.stoppoint  
intrinsic. The DbgStopPointInst class "matches" in this case, allowing
you to use code like this:

if (DbgStopPointInst *I = dyn_cast< DbgStopPointInst>(someinst)) {
    print(I->getLine());
}

Where getLine() is a very nice helper function.

What do you think about making BuildVectorSDNode work exactly like  
this?  That way, BUILD_VECTOR nodes are just normal SDNodes, but we  
can still use:

if (BuildVectorSDNode *BV = dyn_cast< BuildVectorSDNode>(N)) { // only  
works if the opcode is ISD::BUILD_VECTOR
   if (BV->isSplat(info,returned, byref))
      do stuff with info.
}

This makes the BuildVectorSDNode just a nice place to put the various  
methods for inspecting build vector but doesn't require the uniquing  
machinery to know about it, and means that things like SplatBits,  
SplatUndef etc aren't instance variables.

What do you think?

Again, I'm am really really really sorry for the confusion, I did not  
intend to derail the entire discussion with nonsense :( :(

-Chris