[PATCH] D88482: [SVE][CodeGen] Replace TypeSize comparison operators with their scalar equivalents
Sander de Smalen via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 30 03:35:49 PDT 2020
sdesmalen added a comment.
>From a fixed-width vector perspective, all the `getFixedSize()` changes are purely NFC. Changes like `getSizeInBits() -> getScalarSizeInBits()` are _possibly_ functional if your assumption that the type must be a scalar value, ends up being false. I think the exception here are the cases where it can be trivially seen that the function bails out early when the type is not a scalar.
>From a scalable-vector perspective, the `getFixedSize()` changes are not necessarily non-functional, because if a scalable vector goes down these code-paths things will now break with an assert where it would previously 'just work'. In your patch you're making the assumption that scalable vectors shouldn't go down these code-paths, so they are likely to be non-functional.
I think Eli's point is that you could move out all the `getFixedSize` changes into a separate patch, as those are known to be NFC for fixed-width vectors and can't break any of the buildbots or impact any other users of LLVM that don't compile for SVE.
================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:7839
+ WideVT =
+ EVT::getVectorVT(*DAG.getContext(), WideVT, VT.getVectorElementCount());
----------------
Was this change supposed to be part of this patch?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88482/new/
https://reviews.llvm.org/D88482
More information about the llvm-commits
mailing list