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

[Edward Jones via Phabricator via llvm-commits]

edward-jones added inline comments.
Herald added a subscriber: sameer.abuasal.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoB.td:510
+  def : Pat<(smin GPR:$rs1, GPR:$rs2), (MIN  GPR:$rs1, GPR:$rs2)>;
+  def : Pat<(riscv_selectcc GPR:$rs1, GPR:$rs2, (i32 20), GPR:$rs1, GPR:$rs2),
+            (MIN  GPR:$rs1, GPR:$rs2)>;
----------------
Is there a way to use a symbolic value for the CC here instead of the (i32 20) magic number? I notice that other backends appear to use "SETLT", "SETULT" and similar in their DAG patterns.


================
Comment at: llvm/test/CodeGen/RISCV/rv32Zbb.ll:13
+; RV32I-LABEL: _ctlz_i32:
+; RV32I:       # %bb.0:
+; RV32I-NEXT:    addi sp, sp, -16
----------------
Checking for the exact codegen for ctlz without the Zbb extension seems like it could be fragile. Is it necessary to test the non-Zbb expansion of ctlz given that it is just the default lowering?

If the test exists to make sure that "clz" isn't generated unless Zbb is present then it might be easier to change this test into a single "RV32I-NOT clz".


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

https://reviews.llvm.org/D67348