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

[Simon Cook via Phabricator via llvm-commits]

simoncook added a comment.

Looking at the structure this is looking good, it's probably worth renaming the codegen tests to match what I did with the MC patch before commiting ('Z'->'z') but other than a few formatting changes, this seems good. I haven't yet read through all the patterns, I'll add more comments as I go through it.



================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:159
+
+  if(!Subtarget.hasStdExtZbb()) {
+    setOperationAction(ISD::CTTZ, XLenVT, Expand);
----------------
missing a space here after if


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:165
+
+  if(Subtarget.hasStdExtZbp()) {
+    setOperationAction(ISD::BITREVERSE, XLenVT, Legal);
----------------
missing a space here after if


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:169
+
+  if(Subtarget.hasStdExtZbt()) {
+    setOperationAction(ISD::FSHL, XLenVT, Legal);
----------------
missing a space here after if


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoB.td:642
+let Predicates = [HasStdExtZbbOrZbp] in {
+  def : Pat<(and GPR:$rs1, (not GPR:$rs2)), (ANDN GPR:$rs1, GPR:$rs2)>;
+  def : Pat<(or  GPR:$rs1, (not GPR:$rs2)), (ORN  GPR:$rs1, GPR:$rs2)>;
----------------
I think the patterns shouldn't be indented after a `let`, the other backend tablegen files don't indent here, so we should be consistent.


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

https://reviews.llvm.org/D67348