[PATCH] D89449: [RISCV] Initial infrastructure for code generation of the RISC-V V-extension

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1503
+
+  switch (VLMul) {
+  default:
----------------
Is VLMul here already in the proper encoding? The cases seem to align with the enum values.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1559
+    // No VL operator in the pseudo, do not modify VL (rd = X0, rs1 = X0).
+    MIB.addReg(RISCV::X0, RegState::Define | RegState::Dead);
+    MIB.addReg(RISCV::X0, RegState::Kill);
----------------
Seems weird to define a reserved register constant register. Are there are other examples of this in RISC-V or another target?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoPseudoV.td:1
+//===-- RISCVInstrInfoRISCVVPseudo.td - RISC-V 'V' Pseudos ------*- tablegen -*-===//
+//
----------------
nit 80 columns


================
Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.td:259
+//
+//		lmul=⅛	lmul=¼	lmul=½	lmul=1	lmul=2	lmul=4	 lmul=8
+//	i64*	N/A	N/A	N/A	nxv1i64	nxv2i64	nxv4i64	 nxv8i64
----------------
Not sure if we should be using these special fraction characters in source files.


================
Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.td:260
+//		lmul=⅛	lmul=¼	lmul=½	lmul=1	lmul=2	lmul=4	 lmul=8
+//	i64*	N/A	N/A	N/A	nxv1i64	nxv2i64	nxv4i64	 nxv8i64
+//	i32	N/A	N/A	nxv1i32	nxv2i32	nxv4i32	nxv8i32	 nxv16i32
----------------
Looks like some formatting of columns was lost here?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D89449