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

qemu: update to 9.1.0 #21540

Merged
merged 1 commit into from
Sep 9, 2024
Merged

qemu: update to 9.1.0 #21540

merged 1 commit into from
Sep 9, 2024

Conversation

heljkon
Copy link
Contributor

@heljkon heljkon commented Aug 1, 2024

@heljkon
Copy link
Contributor Author

heljkon commented Aug 2, 2024

For the clang64 build failure: https://gitlab.com/qemu-project/qemu/-/issues/2476

@heljkon
Copy link
Contributor Author

heljkon commented Aug 14, 2024

@lazka @Biswa96 @jeremyd2019
Based on ticket Regression 9.1.0-rc0: Msys2/Clang64 build fails in QEMU repository a discussion came up about the fitness of CLANG64/CLANGARM64 for QEMU.

thuth, a core QEMU developer stated:

Ok, so Clang does not support gcc_struct yet. You should not use it to build QEMU on MSYS2. While 95% of such a build might work fine, there are likely some emulated devices where this gcc_struct attribute makes a difference and those won't work in such a build.

As I am only contributing to msys2, I ask you the members of msys2 for discussion and decision.

@lazka
Copy link
Member

lazka commented Aug 15, 2024

If upstream doesn't want to fix their struct packing to not depend on gcc_struct there is not much we can do I guess.

Not sure, your call.

I looked at why they added it in the first place: https://lists.gnu.org/archive/html/qemu-devel/2011-08/msg01877.html

@jeremyd2019
Copy link
Member

thuth, a core QEMU developer stated:

Ok, so Clang does not support gcc_struct yet. You should not use it to build QEMU on MSYS2. While 95% of such a build might work fine, there are likely some emulated devices where this gcc_struct attribute makes a difference and those won't work in such a build.

@mstorsjo are you aware of this limitation? I figured clang has to support both struct packing styles, since it emulates both gcc and msvc frontends, but I guess maybe not both on the same target?

@mstorsjo
Copy link
Contributor

thuth, a core QEMU developer stated:

Ok, so Clang does not support gcc_struct yet. You should not use it to build QEMU on MSYS2. While 95% of such a build might work fine, there are likely some emulated devices where this gcc_struct attribute makes a difference and those won't work in such a build.

@mstorsjo are you aware of this limitation? I figured clang has to support both struct packing styles, since it emulates both gcc and msvc frontends, but I guess maybe not both on the same target?

Yes, I'm aware of this. It's indeed a case where Clang does support both, it just doesn't support the attribute. It does IIRC support ms_struct for opting in to the MS struct layout when building for a target that otherwise uses the default layout, but it doesn't yet support gcc_struct for requesting the reverse, when building in a configuration where the MS layout is the default. See llvm/llvm-project#24757 for the general tracking issue, and llvm/llvm-project#71148 for a recent PR that attempts to implement this.

@heljkon
Copy link
Contributor Author

heljkon commented Aug 23, 2024

@lazka @Biswa96 @jeremyd2019 @mstorsjo
Upstream has added a compiler test to make sure, CLANG64 is not build. I don't want to patch this, so I've disabled clang builds.

==> Starting build()...
...
../qemu-9.1.0-rc3/meson.build:321:4: ERROR: Problem encountered: Your compiler does not support __attribute__((gcc_struct)) - please use GCC instead of Clang

A full log can be found at .../mingw-w64-qemu/src/build-CLANG64/meson-logs/meson-log.txt

ERROR: meson setup failed

==> ERROR: A failure occurred in build().
    Aborting...

.../mingw-w64-qemu/src/build-CLANG64/meson-logs/meson-log.txt:

-----------
Command line: `clang++ -m64 ../tmpud8icv9j/testfile.cpp -o ../tmpud8icv9j/output.obj -c -march=nocona -msahf -mtune=generic -O2 -pipe -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector-strong -D_FILE_OFFSET_BITS=64 -O0 -fpermissive -Werror=implicit-function-declaration -Werror=unknown-warning-option -Werror=unused-command-line-argument -Werror=ignored-optimization-argument -std=gnu++11` -> 0
Running compile:
Working directory:  ./
Code:
struct x { int y; } __attribute__((gcc_struct));
-----------
Command line: `clang++ -m64 ../tmpoo9iwq6a/testfile.cpp -o ../tmpoo9iwq6a/output.obj -c -march=nocona -msahf -mtune=generic -O2 -pipe -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector-strong -D_FILE_OFFSET_BITS=64 -O0 -fpermissive -Werror=implicit-function-declaration -Werror=unknown-warning-option -Werror=unused-command-line-argument -Werror=ignored-optimization-argument -std=gnu++11 -Werror` -> 1
stderr:
../tmpoo9iwq6a/testfile.cpp:1:36: error: unknown attribute 'gcc_struct' ignored [-Werror,-Wunknown-attributes]
    1 | struct x { int y; } __attribute__((gcc_struct));
      |                                    ^~~~~~~~~~
1 error generated.
-----------

../qemu-9.1.0-rc3/meson.build:321:4: ERROR: Problem encountered: Your compiler does not support __attribute__((gcc_struct)) - please use GCC instead of Clang

@jeremyd2019
Copy link
Member

That's too bad - my "default" has been using CLANG64 qemu, generally with whpx but sometimes with tcg for arm64 target. I suppose I'll have to switch to UCRT64 until clang gains support for gcc_struct.

@heljkon heljkon mentioned this pull request Aug 30, 2024
7 tasks
@heljkon heljkon marked this pull request as ready for review September 5, 2024 03:25
@heljkon
Copy link
Contributor Author

heljkon commented Sep 5, 2024

@lazka @jeremyd2019 @Biswa96
Please review!

As already discussed, QEMU packages for clang weren't build, because upstream has introduced a test which currently results in failing clang builds:

ERROR: Problem encountered: Your compiler does not support attribute((gcc_struct)) - please use GCC instead of Clang

@heljkon
Copy link
Contributor Author

heljkon commented Sep 9, 2024

@lazka @jeremyd2019 @Biswa96 @MehdiChinoune
Just a reminder: still waiting for review.

@lazka lazka merged commit 30f86a7 into msys2:master Sep 9, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants