[PATCH] D132784: [AArch64][TTI] Set the cost of XTN to 1 for 2xi64 (to 2xi32) and 8xi16 (to8xi8).

Mingming Liu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 29 10:53:03 PDT 2022


mingmingl added a comment.

In D132784#3754849 <https://reviews.llvm.org/D132784#3754849>, @dmgreen wrote:

> This certainly sounds sensible.  There are some tests in llvm/test/Analysis/CostModel/AArch64/cast.ll that suggest these costs are already 1, but that seems to not apply in all situations.

This is a fairly good point.  With 'gdb opt' debugging, the //direct// cause that costs are already 1 is that`cast.ll` doesn't explicitly specify a data-layout (that provides legal integer types) and `opt` won't guess from `mtriple` either. D132856 <https://reviews.llvm.org/D132856> sets the data-layout to aarch64 le <https://github.com/llvm/llvm-project/blob/a11ec00afea327419ec1ab7c78ba6818d6c5bbf7/clang/lib/Basic/Targets/AArch64.cpp#L895> and updated tests passed.  //(not 100% sure if opt should make this 'guess', or data-layout should be mandatory in tests; the latter was discussed in this llvm-dev thread <https://groups.google.com/g/llvm-dev/c/MRot-oWmV6Q/m/zE8Bn-aWZ2MJ>//)

The longer story is, the code executes in the following flow without a data-layout

1. `AArch64TTIImpl::getCastInstrCost` falls back to call `BaseT::getCastInstrCost` (callsite <https://github.com/llvm/llvm-project/blob/9af0a142e43625cb8478b83692510a5abd39f808/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp#L1876>) after aarch64-specific cost-table look-up gives no result.
2. `BaseT::getCastInstrCost` first calls `BaseT::getCastInstrCost` (callsite <https://github.com/llvm/llvm-project/blob/a11ec00afea327419ec1ab7c78ba6818d6c5bbf7/llvm/include/llvm/CodeGen/BasicTTIImpl.h#L967>) and gets cost '1'  so it continues at line 970 <https://github.com/llvm/llvm-project/blob/a11ec00afea327419ec1ab7c78ba6818d6c5bbf7/llvm/include/llvm/CodeGen/BasicTTIImpl.h#L970>
  - Missing data-layout in `cast.ll` means `DataLayout:: LegalIntWidths` vector is empty and thereby no integer type is legal, so 'trunc' is not free <https://github.com/llvm/llvm-project/blob/86b22d312053f38c7ea94af49dd0e93c660ffec8/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h#L553> and this is where 1 is returned <https://github.com/llvm/llvm-project/blob/a11ec00afea327419ec1ab7c78ba6818d6c5bbf7/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h#L558> -> note this is NOT the '1' observed by `llvm/test/Analysis/CostModel/AArch64/cast.l` (3 explains why)

3. `BaseT::getCastInstrCost` continues to get the legalization cost (which is the `1` relied on by `cast.ll`), and returns here <https://github.com/llvm/llvm-project/blob/a11ec00afea327419ec1ab7c78ba6818d6c5bbf7/llvm/include/llvm/CodeGen/BasicTTIImpl.h#L1033>

> I don't really understand why at the moment, I would guess that something is incorrectly marking it as free, but only some of the time.
>
> Can you add a test case to show where it does give a difference in the costs?

Of course!  Created D132856 <https://reviews.llvm.org/D132856> to show what a passing test looks like with aarch64 little-endian layout specified, and updated this patch to show the diff of added entries.

As a collateral result of specifying data-layout and added entries (in this patch), 'trunc <16 x i16> undef to <16 x i8>' cost now becomes 3

- 3 is calculated by cost model as, 1 for split cost, and 2 x cast cost, relevant lines <https://github.com/llvm/llvm-project/blob/a11ec00afea327419ec1ab7c78ba6818d6c5bbf7/llvm/include/llvm/CodeGen/BasicTTIImpl.h#L1084-L1086>)
- https://gcc.godbolt.org/z/rjbdf7Gjd shows `uzp1` (latency 2 and throughput 2 on neoverse-n1) is generated.

I wonder if the fact that `trunc <16 x i16> undef to <16 x i8>` is over-costly indicates an entry "{ ISD::TRUNCATE, MVT::v16i8, MVT::v16i16, 1 }," should be added in this same patch?


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

https://reviews.llvm.org/D132784



More information about the llvm-commits mailing list