updating README examples with Make changes

[debs-sifive]

Related issue: #2321

Type of change: documentation fix

Impact: no functional change

Development Phase: implementation

Release Notes

[colinschmidt]

Is there no plan to revive this nice-to-have feature?

[debs-sifive]

Not that I am aware of

[colinschmidt]

Bummer. Would a PR adding the feature be accepted?

[debs-sifive]

I don't see why not. Might make more sense/be easier to create a different environment variable in the Makefiles than to change the generator flow, imo.

[colinschmidt]

I know I was too slow to review that other PR, but what was the benefit to changing the API?

Also I don't see documentation for the change of multiple command line configs moving from separation by _ to separation by ,

[debs-sifive]

I'm gonna let @seldridge explain that one, I'm not certain that one way is better than the other.

Is config separation currently documented somewhere I should update? Or should I just add something to the README?

[seldridge]

@colinschmidt: I suggested that @debs-sifive go in this direction for the reason that _ is an allowable character in a configuration name while , is not. Namely, I could have a config of My_Foo_Config which is an allowable Scala class, but would parse into three configs.

There are unrelated, expedient benefits of scopt natively parsing comma delineated arguments, but requiring custom parsing for _ delineated.

Was there a reason why _ was used originally?

[seldridge]

And, sorry. My suggestion, I expect, broke your stuff...

[debs-sifive]

I think @colinschmidt is more asking about why we decided to combine project and class name for the configs

[colinschmidt]

I honestly care less about the _ and more about the fact that make CONFIG=DefaultConfig no longer works.

[seldridge]

I think the makefiles could be changed to support this again if needed.

The underlying reason for the change is that Rocket Chip generation is now governed by two main annotations: TopModuleAnnotation(clazz: Class[_ <: Any]) and ConfigsAnnotation(a: Seq[String]). You combine these to start elaboration.

To generate these, the parsing happens without state, i.e., there is a method of A => AnnotationSeq that converts a command line option (of some type A, e.g., String) into one or more annotations. This is not done with knowledge of the existing AnnotationSeq. Hence, there is no way for the option parsing logic to know what the project package is. (It could be specified by another annotation, but the process of converting the command line arguments to annotations is unaware of it.)

I think that #2274 elected to make the mapping simple from CONFIG=Foo to --configs=foo instead of mangling this with the project name. The makefiles could do this, however.

[colinschmidt]

Ok. I think I agree that its better to do this in the makefile rather than in a new annotation.