dft: fix crossplatform inconsistencies #6509
Conversation
donn
force-pushed
the
dft_os_inconsistency
branch
from
80ed7ea to
68e0a69
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
donn
force-pushed
the
dft_os_inconsistency
branch
from
bb912d2 to
bfe369a
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
|
clang-tidy review says "All clean, LGTM! 👍" |
donn
force-pushed
the
dft_os_inconsistency
branch
2 times, most recently
from
2967e18 to
56edc63
Compare
donn
force-pushed
the
dft_os_inconsistency
branch
from
756ab89 to
df45145
Compare
* dft::ScanArchitect: Use a stable sort for scan cells within buckets. Also added a debugPrint to preview the buckets. * This is necessary as otherwise two scan elements with identical bit widths and clock domains's order is undefined and a place for inconsistency between platforms. * dft::ClockDomain::getClockDomainId: Uses the FNV1a hash algorithm which unlike std::hash is not implementation-dependent and will return an identical hash across all platforms. * This is necessary as C++ maps sort not by in the insertion order, but by the key order, e.g., the hash's absolute value. Yep. * dft::GetClockDomainHashFn: Now simply uses `getClockDomainId` for the NoMix mode. * To avoid code duplication. * Updated DFT tets to reflect new sort ordinals. --- DFT tests confirmed returning an identical result on both macOS and Linux on aarch64. Signed-off-by: Mohamed Gaber <[email protected]>
donn
force-pushed
the
dft_os_inconsistency
branch
from
df45145 to
1a20416
Compare
|
@maliberty boost::hash_combine assumes you're using boost::hash which is implementation-dependent, and boost::hash_detail::hash_mix doesn't appear to be in the version of boost the CI builds with. I've just left it as an XOR. |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
std::hash is used throughout OR so I'm curious why only this one place needs fixing. This feels like either a deeper issue or a very small Band-Aid. |
|
@maliberty ScanArchitect uses std::map, which will sort the clock domains, and thus the chains, by the absolute value of the hash. |
What map are you referring to? Your answer only potentially explains why this change is needed and doesn't address my more general question. |
getClockDomainIdfor the NoMix mode.DFT tests confirmed returning an identical result on both macOS and Linux on aarch64.
Resolves #6411