[PATCH] D56907: [TTI] Add generic UADDSAT/USUBSAT and UADDO/USUBO costs

Wed Jan 23 13:24:27 PST 2019

RKSimon marked an inline comment as done.
RKSimon added inline comments.

================
Comment at: test/Analysis/CostModel/X86/arith-usat.ll:313
+; AVX2-NEXT:  Cost Model: Found an estimated cost of 3 for instruction: %V2I64 = call <2 x i64> @llvm.usub.sat.v2i64(<2 x i64> undef, <2 x i64> undef)
+; AVX2-NEXT:  Cost Model: Found an estimated cost of 3 for instruction: %V4I64 = call <4 x i64> @llvm.usub.sat.v4i64(<4 x i64> undef, <4 x i64> undef)
+; AVX2-NEXT:  Cost Model: Found an estimated cost of 6 for instruction: %V8I64 = call <8 x i64> @llvm.usub.sat.v8i64(<8 x i64> undef, <8 x i64> undef)
----------------
nikic wrote:
> RKSimon wrote:
> > RKSimon wrote:
> > > nikic wrote:
> > > > Not directly related to this change, but the current icmp cost model for X86 doesn't distinguish comparison types, so it assigns the same cost to signed and unsigned vector comparisons, even though the latter only exist since AVX512. The cost here should probably be 5 to account for the sign flips.
> > > https://bugs.llvm.org/show_bug.cgi?id=40376
> > Vector select costs are missing as well.
> Unfortunately the comparison cost modeling improvements you implemented end up not applying here, because we don't have an instruction available :( But that doesn't look so simple to fix (would need to change the API to explicitly pass the predicate for that).
Are you talking about PR40376 ? That work is still going on and its possibly a massive yak to shave. I don't see why that should stop us getting on with this patch.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D56907