llvm dev - Dec 2009 - [LLVMdev] SplitVecRes with SIGN_EXTEND_INREG unsupported

Thanks Eli,
 I'll see if I can get something working and submit a patch.


Micah

-----Original Message-----
From: Eli Friedman [mailto:eli.friedman at gmail.com] 
Sent: Wednesday, December 09, 2009 11:18 PM
To: Villmow, Micah
Cc: llvmdev at cs.uiuc.edu
Subject: Re: [LLVMdev] SplitVecRes with SIGN_EXTEND_INREG unsupported

On Wed, Dec 9, 2009 at 8:40 PM, Villmow, Micah <Micah.Villmow at amd.com>
wrote:
> I have code that is generating sign extend in reg on a v8i32, but the
> backend does not support this data type.  This then asserts in
> LegalizeVectorTypes.cpp:389 because there is no function to split this
> vector into smaller sizes. Would a correct solution be to add this case so
> to trigger the SplitVecRes_BinaryOp function?
SIGN_EXTEND_INREG isn't a binary operation; the correct expansion is
expanding the first operand the same way SplitVecRes_BinaryOp does,
while passing through the second operand untouched.  But yes, adding a
case to DAGTypeLegalizer::SplitVectorResult is the right idea.

If anyone else is curious, here's a testcase which crashes on x86 when
llc is run over it:
define <8 x i32> @a(<8 x i32> %a) {
  %b = trunc <8 x i32> %a to <8 x i16>
  %c = sext <8 x i16> %b to <8 x i32>
  ret <8 x i32> %c
}

-Eli

Eli,
 I have a simple SplitVecRes function that implements what you mentioned,
splitting the LHS just as in BinaryOp, but passing through the RHS. The problem
is that the second operand is MVT::Other, but when casted to an VTSDNode reveals
that it is a vector length of the same size as the LHS SDValue. This causes a
split on the LHS side to work correctly, but then it fails instruction selection
because of Other. I have not been able to figure out how to split the MVT::Other
node yet, any idea how to do this?

Instruction selection fails because it cannot match the pattern:
v4i8 = sign_extend_inreg <v4i8 node>, <Other node(v8i8 VTSDNode)>

The reason being my initial implementation based on the advice given takes as
input:
v8i8 = sign_extend_inreg <v8i8 node>, <Other node(v8i8 VTSDNode)>
and generates two:
v4i8 = sign_extend_inreg <v4i8 node>, <Other node(v8i8 VTSDNode)>

Instead it should generate:
v4i8 = sign_extend_inreg <v4i8 node>, <Other node(v4i8 VTSDNode)>

So, how would I be able to split the Other node so that it will match the
resulting data type?

My function looks like this:
void DAGTypeLegalizer::SplitVecRes_SIGN_EXTEND_INREG(SDNode *N, SDValue &Lo,
                                         SDValue &Hi) {
  SDValue LHSLo, LHSHi;
  GetSplitVector(N->getOperand(0), LHSLo, LHSHi);
  SDValue RHS = N->getOperand(1);
  DebugLoc dl = N->getDebugLoc();

  Lo = DAG.getNode(N->getOpcode(), dl, LHSLo.getValueType(), LHSLo, RHS);
  Hi = DAG.getNode(N->getOpcode(), dl, LHSHi.getValueType(), LHSHi, RHS);
}
Thanks,
Micah
> -----Original Message-----
> From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at
cs.uiuc.edu]
> On Behalf Of Villmow, Micah
> Sent: Thursday, December 10, 2009 9:58 AM
> To: Eli Friedman
> Cc: llvmdev at cs.uiuc.edu
> Subject: Re: [LLVMdev] SplitVecRes with SIGN_EXTEND_INREG unsupported
> 
> Thanks Eli,
>  I'll see if I can get something working and submit a patch.
> 
> 
> Micah
> 
> -----Original Message-----
> From: Eli Friedman [mailto:eli.friedman at gmail.com]
> Sent: Wednesday, December 09, 2009 11:18 PM
> To: Villmow, Micah
> Cc: llvmdev at cs.uiuc.edu
> Subject: Re: [LLVMdev] SplitVecRes with SIGN_EXTEND_INREG unsupported
> 
> On Wed, Dec 9, 2009 at 8:40 PM, Villmow, Micah <Micah.Villmow at
amd.com>
> wrote:
> > I have code that is generating sign extend in reg on a v8i32, but the
> > backend does not support this data type.  This then asserts in
> > LegalizeVectorTypes.cpp:389 because there is no function to split
> this
> > vector into smaller sizes. Would a correct solution be to add this
> case so
> > to trigger the SplitVecRes_BinaryOp function?
> 
> SIGN_EXTEND_INREG isn't a binary operation; the correct expansion is
> expanding the first operand the same way SplitVecRes_BinaryOp does,
> while passing through the second operand untouched.  But yes, adding a
> case to DAGTypeLegalizer::SplitVectorResult is the right idea.
> 
> If anyone else is curious, here's a testcase which crashes on x86 when
> llc is run over it:
> define <8 x i32> @a(<8 x i32> %a) {
>   %b = trunc <8 x i32> %a to <8 x i16>
>   %c = sext <8 x i16> %b to <8 x i32>
>   ret <8 x i32> %c
> }
> 
> -Eli
> 
> 
> 
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

On Thu, Dec 10, 2009 at 12:46 PM, Villmow, Micah <Micah.Villmow at
amd.com> wrote:
> Eli,
>  I have a simple SplitVecRes function that implements what you mentioned,
splitting the LHS just as in BinaryOp, but passing through the RHS. The problem
is that the second operand is MVT::Other, but when casted to an VTSDNode reveals
that it is a vector length of the same size as the LHS SDValue. This causes a
split on the LHS side to work correctly, but then it fails instruction selection
because of Other. I have not been able to figure out how to split the MVT::Other
node yet, any idea how to do this?
You should be able to split the contained type with GetSplitDestVTs,
then recreate the node using SelectionDAG::getValueType(), I think.

That said, it could possibly be considered a bug in DAGCombine that
the second operand is a vector type; someone want to comment on that?

-Eli