This is an archive of the discontinued LLVM Phabricator instance.

[RISCV][docs] Add some wording around vendor extensions
ClosedPublic

Authored by reames on Oct 28 2022, 11:40 AM.

Details

Summary

This adds an initial bit of policy around inclusion of vendor extensions. My intention here is to leave all of the actual decision making to a case by case decision on the regular sync calls, but to spell out some of the pieces we've discussed and (I think) have general agreement on.

Diff Detail

Event Timeline

reames created this revision.Oct 28 2022, 11:40 AM
reames requested review of this revision.Oct 28 2022, 11:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 28 2022, 11:40 AM
craig.topper added inline comments.Oct 31 2022, 11:25 AM
llvm/docs/RISCVUsage.rst
154

I think it's hypenated as "Non-Standard" in the spec.

reames updated this revision to Diff 472188.Oct 31 2022, 5:13 PM

Match standard usage of hyphen and capitalization.

This revision is now accepted and ready to land.Oct 31 2022, 9:22 PM
asb added a comment.Nov 1 2022, 3:29 AM

Two suggestions, but otherwise looks good to me. Thanks!

llvm/docs/RISCVUsage.rst
152

This is the first and only reference to "RVI" in this document. It would be better to spell it out as "RISC-V International"

156

"for discussion" -> "for initial discussion, which may lead to a request to write up an RFC on LLVM's Discourse".

We don't have a policy here yet - I suspect it might not be the case that _every_ vendor extension needs an RFC (e.g. cases where there's a small number of instructions supported only at the MC layer), but I think it would be good to flag that there are cases where broader LLVM developer buy-in might be necessary, and also make it clear that nothing in this document implies discussion on the sync-up calls bypasses that.

This revision was automatically updated to reflect the committed changes.
reames marked an inline comment as done.Nov 1 2022, 9:36 AM
reames added inline comments.
llvm/docs/RISCVUsage.rst
152

Good catch, done.

156

I landed without this change because I didn't feel the wording you suggested clarified meaning. I understand your point, but "for discussion" doesn't imply anything to me about it being a final discussion or otherwise bypassing usual decision making.

I'm open to trying to clarify this if you think it's needed, but I think it probably needs phrased differently.

asb added inline comments.Nov 1 2022, 9:45 AM
llvm/docs/RISCVUsage.rst
156

I can't immediately think of better phrasing and it's not a big deal either way. I think you see what I'm trying to clarify but I agree attempts to do so perhaps just make it more verbose and less clear.