[PATCH] D56907: [TTI] Add generic USUBSAT costs

[Simon Pilgrim via Phabricator via llvm-commits]

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


================
Comment at: include/llvm/CodeGen/BasicTTIImpl.h:1278
+      Cost += ConcreteTTI->getCmpSelInstrCost(BinaryOperator::ICmp, RetTy,
+                                              CondTy, nullptr);
+      Cost += ConcreteTTI->getCmpSelInstrCost(BinaryOperator::Select, RetTy,
----------------
nikic wrote:
> Should we omit the icmp cost for the scalar case, on the assumption that it will be handled as an overflow flag instead?
Maybe just add those as x86 entries for now unless you want to add cost support for add/sub overflow/carry intrinsics as well?


================
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)
----------------
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.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56907