-
Notifications
You must be signed in to change notification settings - Fork 4
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
WIP: Build stage 2 GCC with plugin support and other minor GCC build improvements #168
base: main
Are you sure you want to change the base?
Conversation
@@ -45,6 +45,7 @@ if [[ "$RUN_CONFIG" = 1 ]] || [[ ! -f "$GCC_BUILD_PATH/Makefile" ]]; then | |||
;; | |||
*mingw*) | |||
TARGET_OPTIONS="$TARGET_OPTIONS \ | |||
--libexecdir=$TOOLCHAIN_PATH/lib \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This option is present in stage 2 build. Although there is no imminent issue without this option, it make thes result of stage 1 build consistent with the stage 2.
@@ -59,6 +59,11 @@ if [[ "$RUN_CONFIG" = 1 ]] || [[ ! -f "$GCC_BUILD_PATH/Makefile" ]]; then | |||
--without-libintl-prefix" | |||
;; | |||
*mingw*) | |||
# ADDED: --enable-plugin | |||
# REMOVED: --with-gmp=$TOOLCHAIN_PATH |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The REMOVED
comments makes explicit what is the difference between this configuration and the upstream MSYS2 package configuration.
@@ -69,6 +74,7 @@ if [[ "$RUN_CONFIG" = 1 ]] || [[ ! -f "$GCC_BUILD_PATH/Makefile" ]]; then | |||
--enable-cloog-backend=isl \ | |||
--enable-version-specific-runtime-libs \ | |||
--enable-lto \ | |||
--enable-plugin \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should not it be enabled by default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For Linux target yes, for MinGW target is must be enabled explicitly as this feature is not used much accros the community. JFYI, MSYS2 cross-compiler packages do not enable this, the MSYS2 native MinGW packages does. That's why I've added the ADDED
comment as I am synchronizing this configuration with the cross-compilation package for now. We may switch to synchronization with the native packages once we will succeed to build them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not change it here. I prefer to leave a comment about requirements in MSYS2 pipeline.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me double check whether there is a difference between number of passing tests with and without this option enabled for cross-compiler testing. If this improves the number of passing tests, I'd keep it enabled for all targets. If it has no effect on the cross-compiler testing results, I'd enable it only for MSYS2 testing. Is this OK for you?
This PR actually contains three separate changes, see in-code comments. IMO it would be overkill to submit it separatelly, but I can do it if needed.