Page MenuHomePhabricator

[RISCV] refactor FeatureRVCHints to make ProcessorModel more intuitive
ClosedPublic

Authored by khchen on Mar 29 2020, 9:08 PM.

Details

Summary

I thought putting the FeatureRVCHints in ProcessorModel is not intuitive. (for example, maybe we will have RV32IHints/RV64IHints instructions)
so I declare the FeatureNoRVCHints feature to achieving the same behavior.

Diff Detail

Event Timeline

khchen created this revision.Mar 29 2020, 9:08 PM
asb added a comment.Apr 9 2020, 5:43 AM

I can see the argument for changing the default of EnableRVCHintInstrs. Might we be better just doing that, and keeping it as "rvc-hints" to avoid adding negative features?

khchen added a comment.Apr 9 2020, 7:49 AM
In D77030#1971795, @asb wrote:

I can see the argument for changing the default of EnableRVCHintInstrs. Might we be better just doing that, and keeping it as "rvc-hints" to avoid adding negative features?

@asb I had tried it but the default enabling feature should put ProcessorModels like D62592 said.
so I reference ARM's FeatureNoNegativeImmediates (ARM.td, ARMPredicates ) to add negative features.

khchen added a comment.Apr 9 2020, 7:18 PM

I agree negative features is not intuitive, but HINT instruction default is legal and supported, in other words, it mean the disabling HINT instruction is a feature, right?
There were also negative features in other backend. (grep FeatureNo) so we are not first support it.
For sure, I won’t insist on doing this, I feel it's just a friendly trade-off between user and developer.
Thanks. :P

khchen updated this revision to Diff 261187.Apr 30 2020, 4:35 AM
khchen edited the summary of this revision. (Show Details)

rebase on master

evandro accepted this revision.Jul 9 2020, 8:06 AM
This revision is now accepted and ready to land.Jul 9 2020, 8:06 AM
asb accepted this revision.Jul 9 2020, 10:50 AM

Let's go for it! Thanks.

This revision was automatically updated to reflect the committed changes.