[PATCH] D60160: [PowerPC] Update P9 vector costs for insert/extract element

Jinsong Ji via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 15 12:12:08 PDT 2019


jsji added a comment.

Some comments related to testcases.



================
Comment at: llvm/test/Analysis/CostModel/PowerPC/p9.ll:2
 ; RUN: opt < %s -cost-model -analyze -mtriple=powerpc64-unknown-linux-gnu -mcpu=pwr7 -mattr=+vsx | FileCheck %s
+; RUN: opt < %s -cost-model -analyze -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr8 -mattr=+vsx | FileCheck --check-prefix=CHECK-P8 %s
 ; RUN: opt < %s -cost-model -analyze -mtriple=powerpc64-unknown-linux-gnu -mcpu=pwr9 -mattr=+vsx | FileCheck --check-prefix=CHECK-P9 %s
----------------
Can we have another NFC patch to rename the check-prefixes to avoid confusion. 

Now `CHECK` default to P7, while `CHECK-P8` is for P8 LE, `CHECK-P9` is for P9 BE, `CHECK-LE` is for P9 LE.

Maybe `CHECK-P7`, `CHECK-P8LE`, `CHECK-P9BE`, `CHECK-P9LE` would be better? 

Also maybe use multiple prefixes to check common ones, so that we don't need too many duplicates.
eg: `--check-prefixes=CHECK,CHECK-P7` for `P7`,
`--check-prefixes=CHECK,CHECK-P8LE` for `P8 LE`.



================
Comment at: llvm/test/Analysis/CostModel/PowerPC/p9.ll:45
 
-define void @test4xi32(<4 x i32> %arg1, <4 x i32> %arg2, <4 x i32>* %arg3) {
+define void @test4xi32(i32 %arg1, <4 x i32>* %arg2) {
 
----------------
Maybe it would be better to move all insert/extract to insert_extract.ll?

And also rename this file to something like `vector_unit.ll` other than `p9.ll`?


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

https://reviews.llvm.org/D60160





More information about the llvm-commits mailing list