Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions eng/common/native/init-compiler.sh
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ if [ -z "$CLR_CC" ]; then
# Set default versions
if [ -z "$majorVersion" ]; then
# note: gcc (all versions) and clang versions higher than 6 do not have minor version in file name, if it is zero.
if [ "$compiler" = "clang" ]; then versions="17 16 15 14 13 12 11 10 9 8 7 6.0 5.0 4.0 3.9 3.8 3.7 3.6 3.5"
if [ "$compiler" = "clang" ]; then versions="18 17 16 15 14 13 12 11 10 9 8 7 6.0 5.0 4.0 3.9 3.8 3.7 3.6 3.5"
Copy link
Member

Choose a reason for hiding this comment

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

@janvorli @akoeplinger this hard-coded list has caused issues with source-build a few times when the distro has a version clang that is not yet in this list. Even on already released branches, we've had to update this list.

I don't know the origins of the list, but may be we can change the logic so it accepts whatever is the version is on the system. If there are some specific version that are known to have issues, we can error out for those.

cc @omajid

Copy link
Member

Choose a reason for hiding this comment

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

Originally, there was no list and the build.sh in the dotnet/coreclr repo had a list of command line options for each clang version we knew was working. First, it was to filter out old versions that were not able to compile our code correctly (clang 3.4 and older). It was not clear at that time if all new versions will work correctly, so we kept the list. Then it was transferred to the current form. The idea was that we would add compilers to the list only after we verify that they produce correct code for the runtime so that people trying to build the runtime on whatever systems they have would not hit build / runtime problems. In the past, it was me verifying everything and then adding a new version here or giving it a green flag.
But maybe the benefit of curating this list is not that big and the pain in trying to build on systems with newer compilers is worse. So I guess I'd be fine with either removing the list or keeping it and just showing a warning message if the compiler is not on this list telling people there may be issues.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, sounds good

Copy link
Member

@tmds tmds Mar 21, 2024

Choose a reason for hiding this comment

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

I'd be fine with either removing the list or keeping it and just showing a warning message if the compiler is not on this list telling people there may be issues.

Of these options, I have a slight preference for the first as it avoids all maintenance of the list.

I've created an issue for this: #14632.
I'll pick it up when I find a moment for it, unless someone else picks it up sooner.

elif [ "$compiler" = "gcc" ]; then versions="13 12 11 10 9 8 7 6 5 4.9"; fi

for version in $versions; do
Expand Down Expand Up @@ -125,8 +125,8 @@ if [ -z "$CC" ]; then
exit 1
fi

# Only lld version >= 9 can be considered stable. lld doesn't support s390x.
if [ "$compiler" = "clang" ] && [ -n "$majorVersion" ] && [ "$majorVersion" -ge 9 ] && [ "$build_arch" != "s390x" ]; then
# Only lld version >= 9 can be considered stable. lld supports s390x starting from 18.0.
if [ "$compiler" = "clang" ] && [ -n "$majorVersion" ] && [ "$majorVersion" -ge 9 ] && ([ "$build_arch" != "s390x" ] || [ "$majorVersion" -ge 18 ]); then
if "$CC" -fuse-ld=lld -Wl,--version >/dev/null 2>&1; then
LDFLAGS="-fuse-ld=lld"
fi
Expand Down