This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Introduce a RISCV CondCode enum instead of using ISD:SET* in MIR. NFC
ClosedPublic

Authored by craig.topper on Aug 3 2021, 1:23 PM.

Details

Summary

Previously we converted ISD condition codes to integers and stored
them directly in our MIR instructions. The ISD enum kind of belongs
to SelectionDAG so that seems like incorrect layering.

This patch instead uses the CondCode node on RISCV::SELECT_CC until
isel and then converts it from ISD encoding to a RISCV specific value.
This value can be converted to/from the RISCV branch opcodes in the RISCV namespace. We
can't use the RISCV namespace branch opcodes directly as that would make the
MIR printed values change when new instructions are added.

My larger motivation is to possibly implement something like this patch
from gcc https://patchwork.ozlabs.org/project/gcc/patch/20190430234741.8120-1-jimw@sifive.com/#2163277
This will require a new pseudo instruction for select that needs to
carry a branch condition and live probably until RISCVExpandPseudos.
Using an ISD encoding in RISCVExpandPseudos doesn't seem like correct
layering.

Diff Detail

Event Timeline

craig.topper created this revision.Aug 3 2021, 1:23 PM
craig.topper requested review of this revision.Aug 3 2021, 1:23 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 3 2021, 1:23 PM
Herald added a subscriber: MaskRay. · View Herald Transcript
luismarques accepted this revision.Aug 8 2021, 3:42 PM

LGTM.

We can't use the branch opcode encodings directly as that would make the MIR printed values change when new instructions are added.

I'm not sure I understand this point. Can you please elaborate?

This revision is now accepted and ready to land.Aug 8 2021, 3:42 PM
craig.topper added a comment.EditedAug 8 2021, 3:52 PM

LGTM.

We can't use the branch opcode encodings directly as that would make the MIR printed values change when new instructions are added.

I'm not sure I understand this point. Can you please elaborate?

I shouldn't have used "encodings" there. I meant the enum values assigned by tablegen like RISCV::BEQ RISCV::BNE, etc.

craig.topper edited the summary of this revision. (Show Details)Aug 8 2021, 3:54 PM
This revision was landed with ongoing or failed builds.Aug 8 2021, 5:30 PM
This revision was automatically updated to reflect the committed changes.