[PATCH] D152821: [RISCV] Add support for Xcvmac extension in CV32E40P

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoXCvmac.td:1
+//===-- RISCVInstrInfoXCvmac.td - CORE-V instructions -------*- tablegen -*-===//
+//
----------------
This line is 1 character longer than 80 characters


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoXCvmac.td:126
+} // Predicates = [HasVendorXCvmac]
\ No newline at end of file

----------------
Please fix the missing newline


================
Comment at: llvm/test/MC/RISCV/corev/mac/invalid.s:5
+cv.mac t0, t1, t2
+# CHECK-ERROR: instruction requires the following: 'Xcvmac' (Multiply-Accumulate)
+
----------------
Please add `{{$}}` to the end of these lines so it checks to the end of the line.


================
Comment at: llvm/test/MC/RISCV/corev/mac/mac.s:1
+# RUN: llvm-mc -triple=riscv32 --mattr=+xcvmac -show-encoding %s \
+# RUN:        | FileCheck %s --check-prefixes=CHECK-ENCODING,CHECK-INSTR
----------------
Can we merge the per instruction tests into a single file? I think that's what's done for most extensions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152821