[llvm] r195364 - [mips][msa] Fix a corner case in performORCombine() when combining nodes into VSELECT.

[Daniel Sanders]

Hi Chris,

There doesn't seem to be an owner for the MIPS backend in CODE_OWNERS.txt. Can you approve merging this commit to the release_34 branch?

Just to give you the context, it's part of the following series of patches:
* r195343 - [mips][msa/dsp] Only do DSP combines if DSP is enabled.
* r195364 - [mips][msa] Fix a corner case in performORCombine() when combining nodes into VSELECT. 
* r195444 - [mips][msa] Float vector constants cannot use ldi.[wd] directly. Bitcast from the appropriate integer vector type.
* r195455, r195456, and r195469 - [mips][msa] Fix corner case for integer constant splats with undef values. 
Once these four patches are committed, an llvm-stress + llc loop can run for over 15 hours without llc crashing (up from a previous best of ~20 seconds).

> -----Original Message-----
> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> bounces at cs.uiuc.edu] On Behalf Of Daniel Sanders
> Sent: 21 November 2013 16:12
> To: llvm-commits at cs.uiuc.edu
> Subject: [llvm] r195364 - [mips][msa] Fix a corner case in
> performORCombine() when combining nodes into VSELECT.
> 
> Author: dsanders
> Date: Thu Nov 21 10:11:31 2013
> New Revision: 195364
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=195364&view=rev
> Log:
> [mips][msa] Fix a corner case in performORCombine() when combining
> nodes into VSELECT.
> 
> Mask == ~InvMask asserts if the width of Mask and InvMask differ.
> The combine isn't valid (with two exceptions, see below) if the widths differ
> so test for this before testing Mask == ~InvMask.
> 
> In the specific cases of Mask=~0 and InvMask=0, as well as Mask=0 and
> InvMask=~0, the combine is still valid. However, there are more appropriate
> combines that could be used in these cases such as folding x & 0 to 0, or x &
> ~0 to x.
> 
> 
> Added:
>     llvm/trunk/test/CodeGen/Mips/msa/llvm-stress-s449609655-simplified.ll
> Modified:
>     llvm/trunk/lib/Target/Mips/MipsSEISelLowering.cpp
> 
> Modified: llvm/trunk/lib/Target/Mips/MipsSEISelLowering.cpp
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/Target/Mips/MipsSEISelLowering.cpp?rev=195364&r1
> =195363&r2=195364&view=diff
> ==========================================================
> ====================
> --- llvm/trunk/lib/Target/Mips/MipsSEISelLowering.cpp (original)
> +++ llvm/trunk/lib/Target/Mips/MipsSEISelLowering.cpp Thu Nov 21
> +++ 10:11:31 2013
> @@ -594,9 +594,11 @@ static SDValue performORCombine(SDNode *
>        Cond = Op0Op0;
>        IfSet = Op0Op1;
> 
> -      if (isVSplat(Op1Op0, InvMask, IsLittleEndian) && Mask == ~InvMask)
> +      if (isVSplat(Op1Op0, InvMask, IsLittleEndian) &&
> +          Mask.getBitWidth() == InvMask.getBitWidth() && Mask ==
> + ~InvMask)
>          IfClr = Op1Op1;
> -      else if (isVSplat(Op1Op1, InvMask, IsLittleEndian) && Mask == ~InvMask)
> +      else if (isVSplat(Op1Op1, InvMask, IsLittleEndian) &&
> +               Mask.getBitWidth() == InvMask.getBitWidth() && Mask ==
> + ~InvMask)
>          IfClr = Op1Op0;
> 
>        IsConstantMask = true;
> @@ -609,9 +611,11 @@ static SDValue performORCombine(SDNode *
>        Cond = Op0Op1;
>        IfSet = Op0Op0;
> 
> -      if (isVSplat(Op1Op0, InvMask, IsLittleEndian) && Mask == ~InvMask)
> +      if (isVSplat(Op1Op0, InvMask, IsLittleEndian) &&
> +          Mask.getBitWidth() == InvMask.getBitWidth() && Mask ==
> + ~InvMask)
>          IfClr = Op1Op1;
> -      else if (isVSplat(Op1Op1, InvMask, IsLittleEndian) && Mask == ~InvMask)
> +      else if (isVSplat(Op1Op1, InvMask, IsLittleEndian) &&
> +               Mask.getBitWidth() == InvMask.getBitWidth() && Mask ==
> + ~InvMask)
>          IfClr = Op1Op0;
> 
>        IsConstantMask = true;
> 
> Added: llvm/trunk/test/CodeGen/Mips/msa/llvm-stress-s449609655-
> simplified.ll
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/test/CodeGen/Mips/msa/llvm-stress-s449609655-
> simplified.ll?rev=195364&view=auto
> ==========================================================
> ====================
> --- llvm/trunk/test/CodeGen/Mips/msa/llvm-stress-s449609655-simplified.ll
> (added)
> +++ llvm/trunk/test/CodeGen/Mips/msa/llvm-stress-s449609655-simplified.l
> +++ l Thu Nov 21 10:11:31 2013
> @@ -0,0 +1,33 @@
> +; RUN: llc -march=mips < %s
> +; RUN: llc -march=mips -mattr=+msa,+fp64 < %s ; RUN: llc -march=mipsel
> +< %s ; RUN: llc -march=mipsel -mattr=+msa,+fp64 < %s
> +
> +; This test is based on an llvm-stress generated test case with
> +seed=449609655
> +
> +; This test originally failed for MSA with a ; "Comparison requires
> +equal bit widths" assertion.
> +; The legalizer legalized ; the <4 x i8>'s into <4 x i32>'s, then a
> +call to ; isVSplat() returned the splat value for <i8 -1, i8 -1, ...>
> +as a 32-bit APInt ; (255), but the zeroinitializer splat value as an
> +8-bit APInt (0). The ; assertion occured when trying to check the
> +values were bitwise inverses of ; each-other.
> +;
> +; It should at least successfully build.
> +
> +define void @autogen_SD449609655(i8) {
> +BB:
> +  %Cmp = icmp ult i8 -3, %0
> +  br label %CF78
> +
> +CF78:                                             ; preds = %CF81, %CF78, %BB
> +  %Sl31 = select i1 %Cmp, <4 x i8> <i8 -1, i8 -1, i8 -1, i8 -1>, <4 x
> +i8> zeroinitializer
> +  br i1 undef, label %CF78, label %CF81
> +
> +CF81:                                             ; preds = %CF78
> +  br i1 undef, label %CF78, label %CF80
> +
> +CF80:                                             ; preds = %CF81
> +  %I59 = insertelement <4 x i8> %Sl31, i8 undef, i32 1
> +  ret void
> +}
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits