[PATCH] D63567: [ARM] Mve vector shuffles

Sjoerd Meijer via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 24 02:06:16 PDT 2019


SjoerdMeijer added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:6888
+
+static bool isLegalMveShuffleOp(unsigned PFEntry) {
+  unsigned OpNum = (PFEntry >> 26) & 0x0F;
----------------
a proper nit: perhaps Mve -> MVE for consistency


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:7191
+      else if (isLegalMveShuffleOp(PFEntry)) {
+        unsigned LHSID = (PFEntry >> 13) & ((1 << 13)-1);
+        unsigned RHSID = (PFEntry >>  0) & ((1 << 13)-1);
----------------
There's a lot of prior art here of magic constants and shifts etc., but this doesn't mean much to me.
It probably makes sense when you're familiar with querying the shuffle table. 
Not sure if it is worth a comment, but from a quick look at code here, that might not be even necessary.


================
Comment at: llvm/test/CodeGen/Thumb2/mve-shuffle.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -O3 -mtriple=thumbv8.1m.main-arm-none-eabi -mattr=+mve.fp %s -o - | FileCheck %s
+
----------------
Do we need tests when we don't have HasMVEInt? Or is that not really useful (or done elsewhere already)?


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

https://reviews.llvm.org/D63567





More information about the llvm-commits mailing list