Page MenuHomePhabricator

[RISCV] Support .option rvc and norvc

Authored by kito-cheng on Apr 19 2018, 11:51 PM.


Diff Detail


Event Timeline

kito-cheng created this revision.Apr 19 2018, 11:51 PM
apazos added inline comments.Apr 20 2018, 11:13 AM
22 ↗(On Diff #143232)

Kito, can you add an instruction before the first .option rvc to make sure the default behavior is unaltered:
if C ext is enabled, it will compress, but if it is disable it will not compress.

apazos added inline comments.Apr 20 2018, 5:52 PM
29 ↗(On Diff #143232)

and add more than one instruction in an option block to show it compresses or not all of instructions in the block.

sabuasal added inline comments.Apr 22 2018, 2:52 PM
1000 ↗(On Diff #143232)

no braces needed.

1012 ↗(On Diff #143232)

no braces needed

1023 ↗(On Diff #143232)

no braces needed

1035 ↗(On Diff #143232)

no braces needed

sabuasal added inline comments.Apr 22 2018, 3:16 PM
21 ↗(On Diff #143232)

This should be pure virtual method that you only implement in your RISCVTsargetELFstreamer and your RISCVASMstreamer. Making it virtual causes extra entries for your vtable.

Look at MCStreamer.h line 62


  • Fix coding style issue.
  • Add more instructions to testcase.
  • Make emitDirectiveOptionRVC and emitDirectiveOptionNoRVC to pure virtual function.
  • Return nullptr rather than RISCVTargetStreamer in createRISCVObjectTargetStreamer, because we have pure virtual function in RISCVTargetStreamer now.
kito-cheng marked 7 inline comments as done.Apr 24 2018, 12:48 AM
asb added a comment.Apr 25 2018, 8:11 AM

Thanks Kito. Could you please add some tests for the new warning/failure cases introduced by this patch? e.g. .option with unrecognised identifier etc.

98–99 ↗(On Diff #143682)

Unnecessary comment - the function call is pretty self-descriptive.

kito-cheng updated this revision to Diff 144828.May 1 2018, 8:22 PM


  • Add testcase for warning/failure cases.
asb accepted this revision.May 3 2018, 6:19 AM

Thanks for adding the test case, this looks good to me now. Final nit: could youplease get rid of the unnecessary comment for RegisterAsmTargetStreamer (

This revision is now accepted and ready to land.May 3 2018, 6:19 AM
asb added a comment.May 3 2018, 7:32 AM

There's a corner case we're probably going to encounter here vs gcc.

In GCC, .option rvc or .option norvc in inline asm will have the effect of enabling/disabling instruction compression from that point onwards. I assume it won't have that effect with LLVM unless you use -save-temps. It might be that we decide this use of inline asm isn't supported by LLVM and people should use appropriate function or module attributes instead. Either way, a test which demonstrates the behaviour would be worthwhile.

asb requested changes to this revision.May 3 2018, 7:34 AM
This revision now requires changes to proceed.May 3 2018, 7:34 AM


  • Add 2 test cases to show .option rvc and .option norvc in inline asm will not effect the code gen.
asb accepted this revision.May 11 2018, 10:29 AM

Thanks Kito! I'll add a comment to the test/CodeGen/RISCV files when committing just to indicate their purpose.

This revision is now accepted and ready to land.May 11 2018, 10:29 AM
This revision was automatically updated to reflect the committed changes.