[PATCH] D63937: [ARM] MVE: allow soft-float ABI to pass vector types.

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:268
     setOperationAction(ISD::STORE, VT, Legal);
+    setOperationAction(ISD::UNDEF, VT, Legal);
 
----------------
What exactly does making UNDEF legal do? Is there a universal pattern to just ignore it? Why is that not the default?


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:299
+    setOperationAction(ISD::BUILD_VECTOR, VT, Custom);
+    setOperationAction(ISD::UNDEF, VT, Legal);
   }
----------------
Also if we make vector float and longs undef legal, should we be doing the same for integer vectors?


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:6682
   // will be legalized.
-  if (ST->hasNEON() && EltSize >= 32) {
+  if (EltSize >= 32) {
     // Do the expansion with floating-point types, since that is what the VFP
----------------
My understanding is that this is (at least part of) the fix for some of the inefficient codegen we see in many of the mve float expansion tests. I would expect a lot of tests to need updating because of it.  You can hopefully just run the update script on all the mve-* files.


================
Comment at: llvm/test/CodeGen/Thumb2/mve-soft-float-abi.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=thumbv8.1m -mattr=+mve -o - %s | FileCheck %s
+
----------------
I think this should be thumbv8.1m.main-none-eabi.


================
Comment at: llvm/test/CodeGen/Thumb2/mve-soft-float-abi.ll:4
+
+define <16 x i8> @vector_add(<16 x i8> %lhs, <16 x i8> %rhs) {
+; CHECK-LABEL: vector_add:
----------------
Can you add some float and half tests too. They should be the same (in terms of calling convention), but are easy enough to add tests for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63937