This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Remove Opcode field from RVInst. Assign Inst{6-0} directly. NFC
ClosedPublic

Authored by craig.topper on Jul 20 2023, 12:59 AM.

Details

Summary

Most places assign Opcode right after assigning every other bit in
Inst. I don't think treating Opcode separately adds much value. It
doesn't hide what bits belong to the opcode since every other bits is
listed.

This makes RVInst consistent with RVInst16 subclasss which already
assign Inst{1-0} directly.

Diff Detail

Event Timeline

craig.topper created this revision.Jul 20 2023, 12:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 20 2023, 12:59 AM
craig.topper requested review of this revision.Jul 20 2023, 12:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 20 2023, 12:59 AM
Herald added subscribers: eopXD, MaskRay. · View Herald Transcript
wangpc accepted this revision.Jul 20 2023, 2:12 AM

LGTM.
I think it makes the instruction encoding much easier to understand directly.

This revision is now accepted and ready to land.Jul 20 2023, 2:12 AM
asb accepted this revision.Jul 20 2023, 3:31 AM

Totally agree with your reasoning, I think this a good improvement. Thanks.

This revision was landed with ongoing or failed builds.Jul 20 2023, 8:18 AM
This revision was automatically updated to reflect the committed changes.