[PATCH] D67348: [RISCV] Add codegen pattern matching for bit manipulation assembly instructions.

Simon Cook via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 19 04:49:39 PDT 2020


simoncook added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoB.td:529
+  def : Pat<(riscv_selectcc GPR:$rs1, GPR:$rs2, (i32 20), GPR:$rs1, GPR:$rs2),
+            (MIN  GPR:$rs1, GPR:$rs2)>;
+  def : Pat<(umin GPR:$rs1, GPR:$rs2), (MINU GPR:$rs1, GPR:$rs2)>;
----------------
nitpick: can you remove these double spaces after instruction names


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoB.td:541
+  def : Pat<(sra (shl GPR:$rs1, (i32 16)), (i32 16)), (SEXTH GPR:$rs1)>;
+}
+
----------------
Looking at the other RISC-V InstrInfo files, we don't indent here and add comments at the closing block indicating which predicates no longer apply, could you update to be consistent


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoB.td:544
+let Predicates = [HasStdExtZbb, IsRV64] in {
+  def : Pat<(and (add GPR:$rs1, GPR:$rs2), 0xFFFFFFFF), (ADDWU GPR:$rs1, GPR:$rs2)>;
+  def : Pat<(and (sub GPR:$rs1, GPR:$rs2), 0xFFFFFFFF), (SUBWU GPR:$rs1, GPR:$rs2)>;
----------------
These are inconsistently rewrapped to avoid hitting 80 columns, can you wrap those that aren't correctly done so


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67348





More information about the llvm-commits mailing list