Page MenuHomePhabricator

[RISCV] Add H extension
ClosedPublic

Authored by kito-cheng on Oct 26 2022, 11:27 PM.

Details

Summary

h was the prefix of multi-letter extension name, but it become a
extension name in later RISC-V isa spec.

Fortunately we don't have any extension really defined is prefixed
with h, so we can just change that.

Diff Detail

Event Timeline

kito-cheng created this revision.Oct 26 2022, 11:27 PM
kito-cheng requested review of this revision.Oct 26 2022, 11:27 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptOct 26 2022, 11:27 PM
asb added a comment.Oct 27 2022, 5:07 AM

Thanks for this patch Kito - I'm thinking that perhaps we erred in not treating the hypervisor instructions (e.g. HLV* and HSV*) as being gated on the H extension, so a sensible follow-on to this patch would be to mark those instructions as requiring FeatureStdExtH - what do you think?

kito-cheng planned changes to this revision.Oct 27 2022, 9:05 AM

Let me do that within this patch :)

Update instructions which are belong H extension now.

kito-cheng added inline comments.Oct 27 2022, 8:48 PM
llvm/test/MC/RISCV/rvi-aliases-valid.s
270

Note: Those testcase are moved to rv32ih-aliases-valid.s, not just removed.

reames added inline comments.Oct 28 2022, 8:32 AM
llvm/docs/RISCVUsage.rst
54

If I'm reading the code right here, we only have assembly support here. Given that, shouldn't this be Assembly Support?

Changes:

  • Update doc, H is support assembly only.
kito-cheng marked an inline comment as done.Oct 28 2022, 6:53 PM
kito-cheng added inline comments.
llvm/docs/RISCVUsage.rst
54

Corrected, thanks :)

kito-cheng marked an inline comment as done.Nov 4 2022, 9:44 AM

ping :)

asb added a comment.Nov 8 2022, 2:38 AM

Thanks Kito, I think my only remaining request would be to add at least some test coverage for using a H extensions when h isn't included in the ISA string. I don't think such tests are handled very cleanly or consistently right now, but adding something to rv32i-invalid.s alongside similar checks would be better than nothing.

Changes:

  • Rebase to main
  • Add negative test.
reames accepted this revision.Jan 9 2023, 8:27 AM

LGTM

This revision is now accepted and ready to land.Jan 9 2023, 8:27 AM
This revision was landed with ongoing or failed builds.Mon, Jan 9, 5:52 PM
This revision was automatically updated to reflect the committed changes.