[PATCH] D70545: [ARM][MVE][Intrinsics] Add MVE VABD intrinsics.
Simon Tatham via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Nov 26 03:30:09 PST 2019
simon_tatham added a comment.
This mostly LGTM: only a handful of nits.
================
Comment at: clang/include/clang/Basic/arm_mve.td:45
let params = T.Usual in {
+def vabdq: Intrinsic<Vector, (args Vector:$a, Vector:$b), (IRInt<"vabd", [Vector]> $a, $b)>;
+}
----------------
Can you wrap this line to 80 columns, please? I've been trying to fit the rest of the file in that width.
================
Comment at: llvm/lib/Target/ARM/ARMInstrMVE.td:1669
+ list<dag> pattern=[]>
+ : MVE_int<iname, suffix, size, pattern> {
----------------
This new template parameter `iname` seems to be redundant, since I can't see anywhere you set it to anything other than `"vabd"`.
================
Comment at: llvm/lib/Target/ARM/ARMInstrMVE.td:2958
+class MVE_VABD_fp<string iname, string suffix, bit size>
+ : MVE_float<iname, suffix, (outs MQPR:$Qd), (ins MQPR:$Qn, MQPR:$Qm),
"$Qd, $Qn, $Qm", vpred_r, ""> {
----------------
Similarly here: `iname` is an unnecessary extra template parameter.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70545/new/
https://reviews.llvm.org/D70545
More information about the cfe-commits
mailing list