[PATCH] D93767: [RISCV] Legalize select when Zbt extension available

[Michael Munday via Phabricator via llvm-commits]

mundaym added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoB.td:780
           (CMIX GPR:$rs1, GPR:$rs2, GPR:$rs3)>;
-def : Pat<(riscv_selectcc GPR:$rs2, (XLenVT 0), (XLenVT 17), GPR:$rs3, GPR:$rs1),
+def : Pat<(select (XLenVT (setne GPR:$rs2, 0)), GPR:$rs1, GPR:$rs3),
+          (CMOV GPR:$rs1, GPR:$rs2, GPR:$rs3)>;
----------------
lenary wrote:
> craig.topper wrote:
> > mundaym wrote:
> > > craig.topper wrote:
> > > > Is there a difference between doing it this way versus just adding more riscv_selectcc patterns for other condition codes?
> > > No I think either would work. Using `riscv_selectcc` would have the advantage that we'd only encounter normalised condition codes so we'd need fewer patterns. Do you have a preference?
> > I think my preference is towards fewer patterns.
> @mundaym and I discussed this today, and fewer patterns makes sense, though there's a slight hiccup with using `CondCode` directly in the `riscv_selectcc` pattern that he's looking into.
After further investigation it looks like using `riscv_selectcc` will end up requiring roughly the same number of patterns because we need to lower each legal comparison operation (`SETEQ`, `SETGE`, etc.) individually whereas with `select` we only need to do it for comparison operations that benefit from optimization.

I think we should therefore stick with the original approach I took of legalizing `select` with some additional optimization patterns. This approach also bypasses the issue @lenary mentioned where the comparison operation (e.g. `SETEQ`) currently has to be encoded as an immediate in `riscv_selectcc` (e.g. `XLenVT 17`) which is a bit fragile, especially if more patterns are added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93767