[PATCH] D79870: [RISCV] Add matching of codegen patterns to RISCV Bit Manipulation Zbb asm instructions

[Paolo Savini via Phabricator via llvm-commits]

PaoloS marked 7 inline comments as done.
PaoloS added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoB.td:641
+def SROIPat  : ComplexPattern<XLenVT, 2, "SelectSROI", [or]>;
+def SLOIWPat  : ComplexPattern<XLenVT, 2, "SelectSLOIW", [sext_inreg]>;
+def SROIWPat  : ComplexPattern<XLenVT, 2, "SelectSROIW", [or]>;
----------------
lewis-revill wrote:
> PaoloS wrote:
> > lewis-revill wrote:
> > > Can these W selects be guarded for 64 bit only?
> > Not sure how to do it, they can't be enclosed in Predicates like the instruction patterns.
> Looks like you fixed this with the operand to `ComplexPattern`. Only nitpick is to get the `:` characters aligned vertically here.
On it.


================
Comment at: llvm/test/CodeGen/RISCV/rv32Zbb.ll:32
+
+define i64 @slo_i64(i64 %a, i64 %b) nounwind {
+; RV32I-LABEL: slo_i64:
----------------
PaoloS wrote:
> lewis-revill wrote:
> > Nitpick: For these tests on RV32 where no bitmanip instructions are selected (EG: `slo_i64`, `sro_i64`, `min_i64` etc.) perhaps it's worth either omitting these, or if the goal is to eventually support them, just add a quick comment?
> > 
> > I noticed the same in the 3rd and 5th patches too, for `rol_i64` and `fshl_i64`.
> I see what you mean. I just like the idea to show consistency with the other tests while showing cases where there's still room for improvement. Much of this codegen pattern-matching work was also to look for cases that could be optimized.
> Also things could still change considering that a new subextension is being drafted. 
> On the other hand I understand that it looks like these tests are wrong I guess since they don't show particular changes.
I'm commenting those.


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

https://reviews.llvm.org/D79870