Re: svn commit: r289221 - head/contrib/llvm/lib/Target/X86

[Ivan Klymenko]

On Tue, 13 Oct 2015 16:24:22 + (UTC)
Dimitry Andric  wrote:

> Author: dim
> Date: Tue Oct 13 16:24:22 2015
> New Revision: 289221
> URL: https://svnweb.freebsd.org/changeset/base/289221
> 
> Log:
>   Pull in r250085 from upstream llvm trunk (by Andrea Di Biagio):
>   
> [x86] Fix wrong lowering of vsetcc nodes (PR25080).
>   

Thanks!
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

svn commit: r289221 - head/contrib/llvm/lib/Target/X86

[Dimitry Andric]

Author: dim
Date: Tue Oct 13 16:24:22 2015
New Revision: 289221
URL: https://svnweb.freebsd.org/changeset/base/289221

Log:
  Pull in r250085 from upstream llvm trunk (by Andrea Di Biagio):
  
[x86] Fix wrong lowering of vsetcc nodes (PR25080).
  
Function LowerVSETCC (in X86ISelLowering.cpp) worked under the wrong
assumption that for non-AVX512 targets, the source type and destination type
of a type-legalized setcc node were always the same type.
  
This assumption was unfortunately incorrect; the type legalizer is not 
always
able to promote the return type of a setcc to the same type as the first
operand of a setcc.
  
In the case of a vsetcc node, the legalizer firstly checks if the first 
input
operand has a legal type. If so, then it promotes the return type of the 
vsetcc
to that same type. Otherwise, the return type is promoted to the 'next legal
type', which, for vectors of MVT::i1 is always a 128-bit integer vector 
type.
  
Example (-mattr=+avx):
  
  %0 = trunc <8 x i32> %a to <8 x i23>
  %1 = icmp eq <8 x i23> %0, zeroinitializer
  
The initial selection dag for the code above is:
  
v8i1 = setcc t5, t7, seteq:ch
  t5: v8i23 = truncate t2
t2: v8i32,ch = CopyFromReg t0, Register:v8i32 %vreg1
t7: v8i32 = build_vector of all zeroes.
  
The type legalizer would firstly check if 't5' has a legal type. If so, 
then it
would reuse that same type to promote the return type of the setcc node.
Unfortunately 't5' is of illegal type v8i23, and therefore it cannot be 
used to
promote the return type of the setcc node. Consequently, the setcc return 
type
is promoted to v8i16. Later on, 't5' is promoted to v8i32 thus leading to 
the
following dag node:
  v8i16 = setcc t32, t25, seteq:ch
  
  where t32 and t25 are now values of type v8i32.
  
Before this patch, function LowerVSETCC would have wrongly expanded the 
setcc
to a single X86ISD::PCMPEQ. Surprisingly, ISel was still able to match an
instruction. In our case, ISel would have matched a VPCMPEQWrr:
  t37: v8i16 = X86ISD::VPCMPEQWrr t36, t25
  
However, t36 and t25 are both VR256, while the result type is instead of 
class
VR128. This inconsistency ended up causing the insertion of COPY 
instructions
like this:
  %vreg7 = COPY %vreg3; VR128:%vreg7 VR256:%vreg3
  
Which is an invalid full copy (not a sub register copy).
Eventually, the backend would have hit an UNREACHABLE "Cannot emit physreg 
copy
instruction" in the attempt to expand the malformed pseudo COPY 
instructions.
  
This patch fixes the problem adding the missing logic in LowerVSETCC to 
handle
the corner case of a setcc with 128-bit return type and 256-bit operand 
type.
  
This problem was originally reported by Dimitry as PR25080. It has been 
latent
for a very long time. I have added the minimal reproducible from that 
bugzilla
as test setcc-lowering.ll.
  
Differential Revision: http://reviews.llvm.org/D13660
  
  This should fix the "Cannot emit physreg copy instruction" errors when
  compiling contrib/wpa/src/common/ieee802_11_common.c, and CPUTYPE is set
  to a CPU supporting AVX (e.g. sandybridge, ivybridge).

Modified:
  head/contrib/llvm/lib/Target/X86/X86ISelLowering.cpp

Modified: head/contrib/llvm/lib/Target/X86/X86ISelLowering.cpp
==
--- head/contrib/llvm/lib/Target/X86/X86ISelLowering.cppTue Oct 13 
12:51:44 2015(r289220)
+++ head/contrib/llvm/lib/Target/X86/X86ISelLowering.cppTue Oct 13 
16:24:22 2015(r289221)
@@ -13573,6 +13573,35 @@ static SDValue LowerVSETCC(SDValue Op, c
DAG.getConstant(SSECC, dl, MVT::i8));
   }
 
+  MVT VTOp0 = Op0.getSimpleValueType();
+  assert(VTOp0 == Op1.getSimpleValueType() &&
+ "Expected operands with same type!");
+  assert(VT.getVectorNumElements() == VTOp0.getVectorNumElements() &&
+ "Invalid number of packed elements for source and destination!");
+
+  if (VT.is128BitVector() && VTOp0.is256BitVector()) {
+// On non-AVX512 targets, a vector of MVT::i1 is promoted by the type
+// legalizer to a wider vector type.  In the case of 'vsetcc' nodes, the
+// legalizer firstly checks if the first operand in input to the setcc has
+// a legal type. If so, then it promotes the return type to that same type.
+// Otherwise, the return type is promoted to the 'next legal type' which,
+// for a vector of MVT::i1 is always a 128-bit integer vector type.
+//
+// We reach this code only if the following two conditions are met:
+// 1. Both return type and operand type have been promoted to wider types
+//by the type legalizer.
+// 2. The original operand type has been promoted to a 256-bit vector.
+//
+// Note that condition 2. only applies for AVX targets.
+SDValue NewOp