[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