[PATCH] D46311: [AArch64] added FP16 vcvth intrinsic support
Sjoerd Meijer via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri May 4 07:11:57 PDT 2018
SjoerdMeijer added inline comments.
================
Comment at: lib/Target/AArch64/AArch64InstrFormats.td:5982
let Predicates = [HasNEON, HasFullFP16] in {
- def #NAME#16 : BaseSIMDThreeScalar<U, {S,0b10}, {0b00,opc}, FPR16, asm,
- []>;
+ def #NAME#16 : BaseSIMDThreeScalar<U, {S,0b10}, {0b00,opc}, FPR16, asm, []>;
} // Predicates = [HasNEON, HasFullFP16]
----------------
Nit: trailing whitespace
================
Comment at: lib/Target/AArch64/AArch64InstrFormats.td:7793
multiclass SIMDFPScalarRShift<bit U, bits<5> opc, string asm> {
+
let Predicates = [HasNEON, HasFullFP16] in {
----------------
Nit: unnecessary new line.
================
Comment at: lib/Target/AArch64/AArch64InstrFormats.td:7816
+
+ def DHr : BaseSIMDScalarShift<U, opc, {?,?,?,?,?,?,?},
+ FPR64, FPR16, vecshiftR64, asm, []> {
----------------
Do the HDr and DHr patterns need to be guarded by predicates [HasNEON, HasFullFP16]? Can you check if they are all predicates are correctly set here? Also looks like we can simply things here a bit: merge all patterns with the same neon and fullfp16 predicates in one block.
================
Comment at: lib/Target/AArch64/AArch64InstrFormats.td:7834
+ let Predicates = [HasNEON, HasFullFP16] in {
+ def h : BaseSIMDScalarShift<U, opc, {0,0,1,?,?,?,?},
+ FPR16, FPR16, vecshiftR16, asm, []> {
----------------
To keep the changes minimal, can you please move the "def h" back up where it was?
Repository:
rL LLVM
https://reviews.llvm.org/D46311
More information about the llvm-commits
mailing list