[PATCH] D132296: [RISCV] Add cost model for compare and select instructions.
Jianjian Guan via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 6 23:10:59 PDT 2022
jacquesguan added inline comments.
================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:730
- // TODO: Add cost for fp vector compare instruction.
+ // Scalarize fp vector.
+ if ((ValTy->getScalarSizeInBits() == 16 && !ST->hasVInstructionsF16()) ||
----------------
reames wrote:
> Scalarize isn't what we're doing here. Fix the comment.
Done.
================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:731
+ // Scalarize fp vector.
+ if ((ValTy->getScalarSizeInBits() == 16 && !ST->hasVInstructionsF16()) ||
+ (ValTy->getScalarSizeInBits() == 32 && !ST->hasVInstructionsF32()) ||
----------------
reames wrote:
> I'm not sure about the correctness of this floating point test. There's too many interacting extensions, and I'm not sure when these predicates get set. Looking at the code, I think the check is right here, but I'm not sure.
>
> @craig.topper What's your take here?
>
> Style wise, I'd encourage you to write this as:
> if (has appropriate type clause) {
> switch (VecPred)
> case OneOfMany:
> case TwoOfMany:
> ...
> case NOfMany:
> return LT.first * 1;
> };
> }
>
> Specifically, let all of the unhandled cases fall through.
>
I changed to switch form.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132296/new/
https://reviews.llvm.org/D132296
More information about the llvm-commits
mailing list