Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dev: Fix std::min type mismatch in reg_bank.hh #582

Merged
merged 1 commit into from
Nov 21, 2023

Conversation

BobbyRBruce
Copy link
Member

@BobbyRBruce BobbyRBruce commented Nov 21, 2023

#386 included two cases in "src/dev/reg_bank.hh" where std:: min was used to compare a an integer of type size_t and another of type Addr. This causes an error on my Apple Silicon Mac as the comparison between an "unsigned long" and an "unsigned long long" is not permitted. To fix this issue this patch changes reg_size from size_t to Addr, as well as it the types of the values it was derived from and the variable used to hold the return from the std::min calls. While not completely correct typing from a labelling perspective (reg_bytes is not an address), functions in "src/dev/reg_bank.hh" already abuse Addr in this way frequently (for example, bytes in the write function).

@BobbyRBruce BobbyRBruce added bug compilation error dev General gem5 development code. Found in "src/dev" labels Nov 21, 2023
@BobbyRBruce
Copy link
Member Author

For the curious, it seems like The C++ stdlib provided by MaxOSX.sdk enforces demands equal type/bitwidth for the integers passed to std:min:

In file included from build/ALL/python/_m5/param_IdeDisk.cc:17:
In file included from src/dev/storage/ide_disk.hh:52:
In file included from src/dev/storage/ide_ctrl.hh:40:
src/dev/reg_bank.hh:984:36: error: no matching function for call to 'min'
          const size_t reg_bytes = std::min(reg_size, bytes - done);
                                   ^~~~~~~~
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/__algorithm/min.h:40:1: note: candidate template ignored: deduced conflicting types for parameter '_Tp' ('size_t' (aka 'unsigned long') vs. 'Addr' (aka 'unsigned long long'))
min(const _Tp& __a, const _Tp& __b)

I'm surprised it doesn't upcast in these cases.

gem5#386 included two cases in
"src/dev/reg_bank.hh" where `std:: min` was used to compare a an integer
of type `size_t` and another of type `Addr`. This cause an error on my
Apple Silicon Mac as this is a comparison between an "unsigned long"
and an "unsigned long long" which (at least on my setup) was not
permitted. To fix this issue the `reg_size` was changed from `size_t` to
`Addr`, as well as it the types of the values it was derived from and
the variable used to hold the return from the `std::min` calls.

Change-Id: I31e9c04a8e0327d4f6f5390bc5a743c629db4746
Copy link
Contributor

@powerjg powerjg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like to use Addr this way, but given the rest of the code does use Addr as a size, I think this is fine.

@BobbyRBruce BobbyRBruce merged commit d772f39 into gem5:develop Nov 21, 2023
36 of 41 checks passed
@BobbyRBruce BobbyRBruce deleted the fix-min-type-mismatch branch November 21, 2023 05:37
BobbyRBruce added a commit to BobbyRBruce/gem5 that referenced this pull request Jun 20, 2024
Introduced in gem5#1234, this caused compilation to faill in Apple Silicon
systems. This bug is the same as gem5#582 where a more detailed explanation
is provided.

Change-Id: If186b43c41df6b1da009dc9409cb6facac79fa4f
BobbyRBruce added a commit to BobbyRBruce/gem5 that referenced this pull request Jun 20, 2024
Introduced in gem5#1234, this caused compilation to fail in Apple Silicon
systems. This bug is the same as gem5#582 where a more detailed explanation
is provided.

Fixed by changing `num_bytes` parameter to `size_t`.

Change-Id: If186b43c41df6b1da009dc9409cb6facac79fa4f
BobbyRBruce added a commit that referenced this pull request Jun 20, 2024
Introduced in #1234, this caused compilation to faill in Apple Silicon
systems. This bug is the same as #582 where a more detailed explanation
is provided.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug compilation error dev General gem5 development code. Found in "src/dev"
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants