The custom expansion of select operations in the RISC-V backend
interferes with the matching of cmov instructions. Legalizing
select when the Zbt extension is available solves that problem.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Target/RISCV/RISCVInstrInfoB.td | ||
---|---|---|
780 | Is there a difference between doing it this way versus just adding more riscv_selectcc patterns for other condition codes? |
llvm/lib/Target/RISCV/RISCVInstrInfoB.td | ||
---|---|---|
780 | 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? |
llvm/lib/Target/RISCV/RISCVInstrInfoB.td | ||
---|---|---|
780 | I think my preference is towards fewer patterns. |
The code generation improvements here are looking good, let's hope they stay with a set of riscv_selectcc patterns.
llvm/lib/Target/RISCV/RISCVInstrInfoB.td | ||
---|---|---|
780 | @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. |
llvm/lib/Target/RISCV/RISCVInstrInfoB.td | ||
---|---|---|
780 | 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. |
LGTM!
I'm going to look into removing more of the xori … 1 which are used by the selects in some of these cases.
We've found some issues in testing, so for the moment I've reverted this patch in rG7c9c2a2ea5e3760d7310309c96c9a4ce41fa4d9b.
@mundaym do you have commit access yet? I think Sam had been committing your previous patches?
@craig.topper No, I don't have commit access yet. (Thanks for reviewing this by the way!)
Is there a difference between doing it this way versus just adding more riscv_selectcc patterns for other condition codes?