[PATCH] D56636: [CodeGen][X86] Expand vector USUBSAT to UMAX+SUB

Nikita Popov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Jan 12 06:07:46 PST 2019


nikic marked 2 inline comments as done.
nikic added inline comments.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:1224
+    if (!HasInt256)
+      setOperationAction(ISD::USUBSAT, MVT::v8i32, Custom);
 
----------------
RKSimon wrote:
> nikic wrote:
> > RKSimon wrote:
> > > LowerADDSAT_SUBSAT supports AVX1 splitting, so why not:
> > > ```
> > > setOperationAction(ISD::USUBSAT, MVT::v8i32, HasInt256 ? Legal : Custom);
> > > ```
> > > 
> > > Also, v4i64  should be ok as well.
> > I've added this to get the splitting, but I don't think v8i32 would be legal, as we need it to expand. Maybe `hasInt256 ? Expand : Custom` would make it more obvious what the intention here is?
> Better for the existing x86 lowering code to handle it IMO - that's what we do for nearly all 256-bit integer code on AVX1 targets.
That's what this is intended to do. v8i32 is custom so lowering splits it into two v4i32s, which then get expanded.

If I change this to `HasInt256 ? Legal : Custom`, then I get:

    LLVM ERROR: Cannot select: t5: v8i32 = usubsat t2, t4

Or am I misunderstanding what you have in mind here?


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56636/new/

https://reviews.llvm.org/D56636





More information about the llvm-commits mailing list