This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Improve assembler missing feature warnings
ClosedPublic

Authored by simoncook on Nov 6 2019, 8:38 AM.

Details

Summary

This adds support for printing improved missing feature error messages
from the assembler, which now indicates which feature caused the parse
to fail.

Diff Detail

Event Timeline

simoncook created this revision.Nov 6 2019, 8:38 AM
simoncook updated this revision to Diff 228073.Nov 6 2019, 8:39 AM

Add diff that has useful context

This looks good to me, the only thing that I'm not sure about is the phrasing of the warnings:

"error: instruction requires the following: RV32I Base Instruction Set"

For a naive user (like me) I think the 'I' could make them focus on the extension as the problem, not the fact that's it's RV32 vs RV64.

This looks good to me, the only thing that I'm not sure about is the phrasing of the warnings:

"error: instruction requires the following: RV32I Base Instruction Set"

For a naive user (like me) I think the 'I' could make them focus on the extension as the problem, not the fact that's it's RV32 vs RV64.

I've used the names of the extensions as per the spec (and the SubtargetFeature) in order to map it back more easily to the spec, a user can search for one of these strings in the spec and find out what it is they are missing.

lenary accepted this revision.Nov 20 2019, 3:05 AM

LGTM! This is a really nice improvement, thanks!

This revision is now accepted and ready to land.Nov 20 2019, 3:05 AM
asb accepted this revision.Nov 21 2019, 3:50 AM

Thanks Simon, LGTM. I did wonder about just saying e.g. "the 'c' instruction set extension", but on balance I can't see it being any easier to understand or more obvious what to change, and it's handy giving a longer description.

This revision was automatically updated to reflect the committed changes.