This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Add cost model for SK_Reverse
AbandonedPublic

Authored by Miss_Grape on May 18 2022, 12:21 AM.

Details

Diff Detail

Event Timeline

Miss_Grape created this revision.May 18 2022, 12:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 18 2022, 12:21 AM
Miss_Grape requested review of this revision.May 18 2022, 12:21 AM
craig.topper added inline comments.May 18 2022, 12:31 AM
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
194

Why do we need a table to return a 1 for every legal scalable vector type? There's gotta be better ways to do that.

I don't believe that 1 is the correct answer for RISC-V though. It's a vid.v, a vrsub.vx, and a vrgather.vv if I remember correctly. That's at least a cost of 3 assuming an optimal implementation of vrgather.vv.

230

This doesn't seem to go with the title of this patch.

246

There are no problems with <vscale x 1 x eltty> in the code generator that I'm aware of. Please explain.

llvm/test/Transforms/LoopVectorize/RISCV/riscv-vector-reverse.ll
4

Shouldn't a reverse test have reverse intrinsics in it? Or did instcombine remove them?

5

Do any of the check lines in this file come from -debug-only=loop-vectorize?

8

Remove unneeded attributes please

Miss_Grape added inline comments.May 18 2022, 12:33 AM
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
230

To ensure that the Load/store's InstructionCost‘s state is set to Valid, when VF.isScalar() == true

craig.topper added inline comments.May 18 2022, 12:41 AM
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
230

Can you put it in a different patch that is applied before this one?

Miss_Grape retitled this revision from [RISCV] Add cost model for SK_Reverse to [WIP] [RISCV] Add cost model for SK_Reverse.May 18 2022, 1:14 AM
Miss_Grape added inline comments.May 18 2022, 2:53 AM
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
194

Why do we need a table to return a 1 for every legal scalable vector type? There's gotta be better ways to do that.

CostTableLookup

230

I add the test case, Pls review: https://reviews.llvm.org/D125866

Why do D125866 need loop vectorization? Is the performance improved?

craig.topper added inline comments.May 19 2022, 9:57 PM
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
194

Since you're returning the same cost for every legal vector type, I think this is equivalent to:

if (LT.second.isScalableVector())
  return LT.first * 1

But I still don't believe 1 is the correct value.

Miss_Grape retitled this revision from [WIP] [RISCV] Add cost model for SK_Reverse to [RISCV] Add cost model for SK_Reverse.
Miss_Grape marked 2 inline comments as done.
Miss_Grape marked 2 inline comments as done.
Miss_Grape added inline comments.May 25 2022, 2:02 AM
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
246

Fixed

llvm/test/Transforms/LoopVectorize/RISCV/riscv-vector-reverse.ll
4
This comment was removed by Miss_Grape.
Miss_Grape abandoned this revision.Aug 17 2022, 1:20 AM