[PATCH] D79164: [CostModel] getCFInstrCost

[Sam Parker via Phabricator via llvm-commits]

samparker marked 4 inline comments as done.
samparker added a comment.

Thanks for taking another look.



================
Comment at: llvm/test/Analysis/CostModel/ARM/cast.ll:1721-1729
+; CHECK-V8M-MAIN-NEXT:  Cost Model: Found an estimated cost of 1 for instruction: %loadi8 = load i8, i8* undef, align 1
+; CHECK-V8M-MAIN-NEXT:  Cost Model: Found an estimated cost of 1 for instruction: %loadi16 = load i16, i16* undef, align 2
+; CHECK-V8M-MAIN-NEXT:  Cost Model: Found an estimated cost of 1 for instruction: %loadi32 = load i32, i32* undef, align 4
+; CHECK-V8M-MAIN-NEXT:  Cost Model: Found an estimated cost of 2 for instruction: %loadv2i8 = load <2 x i8>, <2 x i8>* undef, align 2
+; CHECK-V8M-MAIN-NEXT:  Cost Model: Found an estimated cost of 4 for instruction: %loadv4i8 = load <4 x i8>, <4 x i8>* undef, align 4
+; CHECK-V8M-MAIN-NEXT:  Cost Model: Found an estimated cost of 8 for instruction: %loadv8i8 = load <8 x i8>, <8 x i8>* undef, align 8
+; CHECK-V8M-MAIN-NEXT:  Cost Model: Found an estimated cost of 2 for instruction: %loadv2i16 = load <2 x i16>, <2 x i16>* undef, align 4
----------------
dfukalov wrote:
> Is there actual need to specify align here? It is not changed in many other lines..
> The same question for a number of bunches below
I'm not sure how these got here, there has been ongoing alignment work so maybe the default alignment is being added when it's missing. I'll rebase this patch and check it out.


================
Comment at: llvm/test/Analysis/CostModel/ARM/mve-gather-scatter-cost.ll:158-161
+
+
+
+
----------------
dfukalov wrote:
> Do wen need theese extra lines? Is their number connected to lines removed before?-)
> The same strange things below
Yeah I will remove these. I think the test file was already formatted oddly which the script has then just added to the weirdness.


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/aarch64-predication.ll:10-15
 ; "optsize" attribute to disable all interleaving calculations.  A cost of 4
 ; for %tmp4 indicates that we would scalarize it's operand (%tmp3), giving
 ; %tmp4 a lower scalarization overhead.
 ;
 ; COST-LABEL:  predicated_udiv_scalarized_operand
+; COST:        LV: Found an estimated cost of 5 for VF 2 For instruction: %tmp4 = udiv i64 %tmp2, %tmp3
----------------
dfukalov wrote:
> The comment for the test explicitly says we need cost of 4 here.
I'm not really sure what causes this, but it at least shouldn't stop the vectorizer from performing the scalarization.


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/extractvalue-no-scalarization-required.ll:11
 
-; CM: LV: Scalar loop costs: 7.
+; CM: LV: Scalar loop costs: 9.
 ; CM: LV: Found an estimated cost of 5 for VF 2 For instruction:   %a = extractvalue { i64, i64 } %sv, 0
----------------
dfukalov wrote:
> It is not clear why these costs (with same changes below) are increased
The cost increases by two because the phi and the br are no longer classed as free for throughput.


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

https://reviews.llvm.org/D79164