[PATCH] D67344: [ARM][MVE] VCTP instruction selection

Mon Sep 9 03:18:28 PDT 2019

dmgreen added a comment.

In D67344#1662801 <https://reviews.llvm.org/D67344#1662801>, @samparker wrote:

> I'm also wondering how this fits in with D67085 <https://reviews.llvm.org/D67085>...

Should be fine so long as they are not loading or storing anything (or can get it to not load or store anything!)

================
Comment at: lib/Target/ARM/ARMInstrMVE.td:3946
+let Predicates = [HasMVEInt] in {
+def : Pat<(int_arm_vctp8 rGPR:$Rn),
+          (v16i1 (MVE_VCTP8 rGPR:$Rn))>;
----------------
Nitpicking: Indent!

================
Comment at: test/CodeGen/Thumb2/mve-vctp.ll:1
+; RUN: llc -mtriple=thumbv8.1m.main -mattr=+mve %s -o - | FileCheck %s
+
----------------
Might as well add -verify-machineinstrs. Should the triple be in full (-none-eabi)? I'm not sure it matters a lot, but may default to linux otherwise.

Also I would just run the update script on a file this size.

================
Comment at: test/CodeGen/Thumb2/mve-vctp.ll:24
+  %res = call <4 x i1> @llvm.arm.vctp32(i32 %arg)
+  ret <4 x i1> %res
+}
----------------
It's probably best to use the <4 x i1> as opposed to returning it (in a vselect or something).

I'm not sure if the calling convention for a v4i1 is super very well defined. It's not defined from C, so only comes up in llvm test and if we can just sidestep that problem...

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

https://reviews.llvm.org/D67344