[PATCH] D95291: [CostModel] Remove VF from IntrinsicCostAttributes

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added a comment.

Thanks for working on this. Last week I was really puzzled when looking at this code and I wasn't sure if there was a good reason for this, so good to see it get fixed!



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp:763
+    break;
+  case Intrinsic::uadd_sat:
+  case Intrinsic::usub_sat:
----------------
Why are you specially calling out uadd_sat and others?


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/intrinsiccost.ll:11
 ; CHECK-COST: Found an estimated cost of 10 for VF 1 For instruction:   %1 = tail call i16 @llvm.sadd.sat.i16(i16 %0, i16 %offset)
-; CHECK-COST: Found an estimated cost of 26 for VF 2 For instruction:   %1 = tail call i16 @llvm.sadd.sat.i16(i16 %0, i16 %offset)
-; CHECK-COST: Found an estimated cost of 58 for VF 4 For instruction:   %1 = tail call i16 @llvm.sadd.sat.i16(i16 %0, i16 %offset)
-; CHECK-COST: Found an estimated cost of 122 for VF 8 For instruction:   %1 = tail call i16 @llvm.sadd.sat.i16(i16 %0, i16 %offset)
+; CHECK-COST: Found an estimated cost of 4 for VF 2 For instruction:   %1 = tail call i16 @llvm.sadd.sat.i16(i16 %0, i16 %offset)
+; CHECK-COST: Found an estimated cost of 1 for VF 4 For instruction:   %1 = tail call i16 @llvm.sadd.sat.i16(i16 %0, i16 %offset)
----------------
Are there changes to these costs because it would previously calculate `scalarization cost + 2 * cost(sadd.sat.i16)` where it can now calculates the cost as `cost(sadd.sat.2i16)`?


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

https://reviews.llvm.org/D95291