Fix modport direction and conflict

rsd-devel / rsd

RSD: RISC-V Out-of-Order Superscalar Processor

Apache License 2.0

934 stars 95 forks source link

Fix modport direction and conflict #12

Closed dalance closed 4 years ago

dalance commented 4 years ago

This PR fixes modport direction and conflict.

DCacheIF The direction of initMSHR, initMSHR_Addr and memAccessResult seems to be wrong.
MulDivUnitIF For example, dataInA is output of both ComplexIntegerExecutionStage and MemoryExecutionStage. The conflict causes warning at synthesis by Design Compiler.

shioyadan commented 4 years ago

Thanks for your fix.

You fixed the modport DCacheArray in the interface DCacheIF, and the modport is used in the module DCacheArray ( https://github.com/rsd-devel/rsd/blob/master/Processor/Src/Cache/DCache.sv#L417 ).

I checked the module DCacheArray, and the fixed signals (initMSHR, initMSHR_Addr and memAccessResult) are not used in this module. So I think that it is correct to remove these signals from the modport. What do you think of it?

Regarding the second fix, pipelines/stages where those signals are written change depending on the microarchitectural configuration of RSD. Other compilers did not warn such warnings and we did not notice them.

dalance commented 4 years ago

I checked the module DCacheArray, and the fixed signals (initMSHR, initMSHR_Addr and memAccessResult) are not used in this module. So I think that it is correct to remove these signals from the modport. What do you think of it?

I guessed the unused signals are reserved for future usage. If not so , they should be removed.

Regarding the second fix, pipelines/stages where those signals are written change depending on the microarchitectural configuration of RSD. Other compilers did not warn such warnings and we did not notice them.

I understand this is not actual conflict, but the warning may cover actual critical warnings. So I think resolving the warning is better.

shioyadan commented 4 years ago

I guessed the unused signals are reserved for future usage. If not so , they should be removed.

These signals were mistakenly added to the modport, so they should be removed. Thanks for your new commit.

I understand this is not actual conflict, but the warning may cover actual critical warnings. So I think resolving the warning is better.

I agree with your opinion. In fact, we recently found that older versions of Design Compiler cause unknown internal errors in logical synthesis regarding these parts. We were very annoyed with these errors...

I will merge your PR soon. Thanks!