Add explict types to fix errors caused by https://github.com/B-Lang-org/bsc/pull/916

[krame505]

No description provided.

[quark17]

This actually seems unrelated the XXX comments about TLM!

The TLM libraries provide specific amba bus interfaces (like AXI) and convert them to an abstracted common interface (TLM), so then modules can be written with a TLM interface and different shims can be used to convert that to AXI etc as needed for the context. The ID field of the TLM is allowed to be a different size than the AXI ID field (I gather?). And so fromAxiId converts from the Bit n of the AXI bus to the Bit m of the TLM bus. Type synonyms are used to extract individual variables from the list of parameters. But, as far as I know, none of the ones used here are the reason for the XXX. I don't understand why your PR changes whether explicit types are need?

Also, you only add explicit types in one file. In other, you just remove the call to fromAxiId. Why is that? why are they resolved differently? Also, it surprises me that removing the call to fromAxiId works -- if it's not needed, then why does the conversion function exist?

[krame505]

I am still having a hard time understanding what is going on with the defines here. It seems that the aliases must be expanding to the same size of Bit vector, so the fromAxiId isn't technically needed? But I'm not sure if that is accidental, or if having the explicit type is better style in these libraries?

I'm not sure how those ended up with different fixes - I changed them to both have explicit types for now.

[quark17]

This is inside an instance of BusPayload (as defined in bsc-contrib/Libraries/Bus/BusDefines.bsv), which has a dependency from a to b, that says for some payload type a, you can create an ID type b. In these two failing files, we're defining an instance that says, for an AXI command with an ID field, extract a TLM ID of the same size. So there should be no ambiguity about whether padding/truncating is needed.

As far as this library goes, cExtend can be removed from fromAxiId in both cases, since that function is also defining a transform that's got the same size parameter on both sides.

However, I think we should extract a simple example of what's going on here, to add to the BSC testsuite and to understand the change. Because what's happening here is that an explicit type is needed on a method of a typeclass instance. But the typeclass declaration says what the types are, so the type should be known to BSC. For an explicit type to make a difference, it must be that BSC is typechecking it implicitly and then only later trying to unify that with the declared type? That's what I'd like to understand, and whether there's something that can be fixed by doing that typecheck better (like, inserting an explicit type when there isn't one, or something). That fix may not need to be done now, but should at least be filed as a bug, so we know for later. I think I can boil this down to a small example -- give me a sec.

My suggestion, then, is to change this PR to just update 2 lines (remove the cExtend in two places) and to add the new boiled-down testsuite case to the testsuite in the other PR (as an expected failure to compile that should, if not fixed).

[krame505]

Closing this, see discussion on B-Lang-org/bsc#916.

[krame505]

No longer needed: with the conditional synonym expansion in B-Lang-org/bsc#916, ctxRedCQType' leaves plain synonyms in instance heads unexpanded, so the AMBA_TLM code compiles without these explicit types. The reduced case is captured as a test in that PR (CExtendSynonym.bsv), along with an ATF variant that still fails (CExtendATF.bsv, GitHub Issue #311). Closing.