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

Math functions already defined for clib4. #149

Merged
merged 1 commit into from
Oct 17, 2023

Conversation

3246251196
Copy link
Contributor

The patch to introduce clib4 should also modify the configure file of libstdc++-v3 so that math functions that are already defined in the C-library of clib4 are not also defined by libstdc++, otherwise, multiple definition errros will occur. This only affect clib4.

@sba1
Copy link
Owner

sba1 commented Oct 7, 2023

These changes should also happen in configure.ac I think so that after auto generation they are still present.

@3246251196
Copy link
Contributor Author

Can you suggest what should be changed in configure.ac?

The change that I am making is analogous to the change made in the case that ${host} is avr*--).

@sba1
Copy link
Owner

sba1 commented Oct 8, 2023

The configure file is generated from configure.ac via autoconf. So all changes to configure have to be done in the origin file. See the other patches.

@3246251196
Copy link
Contributor Author

3246251196 commented Oct 8, 2023

@sba1
The issue here is that when you originally clone gcc - even if you do gild checkout gcc 11 --skip-patches - a configure file already exists. I see that in the process of building the compiler nothing triggers a regeneration of the configure file. I proved this by modifying configure.ac and saw that no update via autoconf was instantiated.

There is already precedent for direct changes to configure files in previous patches.

Regardless, I always prefer a cleaner approach. I would prefer also to put them into configure.ac but I cannot see how that is going to help; I may very well be missing something.

@sba1
Copy link
Owner

sba1 commented Oct 8, 2023

If there is a configure-only change (without the same being done in configure.ac) then I think it is a bug in the patches. The patches must contain, both the configure.ac and configure changes. It is common to have a configure.ac and the configure script (as generated by autoconf or autoreconf) in source code distributions even if the latter is being built from the former. This is nothing new to adtools gcc.

The idea is to modify configure.ac and to autoreconfig it and then to create patch to align with the standard way of distribution source code. Note that you must use the same autotools version as it has been done by the gcc maintaines for the particular release.

@3246251196
Copy link
Contributor Author

@sba1
Makes sense. Let me get back to you soon.

@3246251196
Copy link
Contributor Author

3246251196 commented Oct 10, 2023

Hi,

So, this is required me to build automake and autoconf from source. Versions 1.15.1 (https://ftp.gnu.org/gnu/automake/automake-1.15.1.tar.gz) and 2.69 (https://ftp.gnu.org/gnu/autoconf/autoconf-2.69.tar.gz) respectively. This was done by just running ./configure --prefix=/some/arbitrary/folder && make && make install.

I then prepended the bin folder of those installs to my path and exported my path.

Of course, I reverted the change in this PR so that the configure file was back to normal. I then added some stuff into crossconfig.m4 because that contains some definitions that are - in turn - used in configure.ac.

From the directory: libstdc++-v3 I ran aclocal ; autoreconf. I saw - almost - the expected change in the newly generated configure file. However, there are a few instances of extra changes to the configure file that I did not expect such as:

Going to:
#line 15967 "configure"
From:
#line 15968 "configure"
Even though the line number of that change is no different.

Going to:
if eval test \"x\$"$as_ac_Header"\" = x"yes"; then :
From:

eval as_val=\$$as_ac_Header
   if test "x$as_val" = x""yes; then :

Going to:
as_fn_error $? "Link tests are not allowed after GCC_NO_EXECUTABLES." "$LINENO" 5
From:
as_fn_error "Link tests are not allowed after GCC_NO_EXECUTABLES." "$LINENO" 5

The output I got from aclocal when running the command was:

configure.ac:13: warning: macro 'AM_ENABLE_MULTILIB' not found in library
configure.ac:95: warning: macro 'AM_PROG_LIBTOOL' not found in library
configure.ac:287: warning: macro 'AM_ICONV' not found in library
configure.ac:266: warning: AC_PROG_LD is m4_require'd but not m4_defun'd
acinclude.m4:180: GLIBCXX_CHECK_LINKER_FEATURES is expanded from...
configure.ac:266: the top level

Do you have any idea why I getting more changes than just the change that I expected?

Regards,
rjd.
@sba1

@sba1
Copy link
Owner

sba1 commented Oct 11, 2023

I think that you must run it in the top-level folder. Also make sure that the versions are really matching. If this doesn't help you could identify where the first change was. It is possible that a file was not regenerated when the patches were ported to a newer version.

@3246251196
Copy link
Contributor Author

Yes, running it in the top level what was I was thinking. I will try again soon. Btw, what if the maintainers used a bunch of options along with aclocal, or autoconf - how am I meant to know. All I am doing is running the tools directly without any options.

@sba1
Copy link
Owner

sba1 commented Oct 11, 2023

Normally there is an autogen.sh file. I had no time to check if this is the case here.

@3246251196
Copy link
Contributor Author

It seems to be an issue somewhere in the list of patches.

When I GILD-clone GCC 11 and --skip-patches, and when I then modify the configure.ac file and then invoke autoreconf -fv I see that the expected change, and only expected change, is generated in the resulting configure script.

At this point, the only thing I can think of is doing the change without the patches, storing those diffs to the configure.ac and configure file, adding all of the GCC 11 patches and then fitting in that original diff.

Can you suggest any other way?

The patch to introduce clib4 should also modify the configure file of
libstdc++-v3 so that math functions that are already defined in the
C-library of clib4 are not also defined by libstdc++, otherwise,
multiple definition errros will occur. This only affect clib4.
@3246251196
Copy link
Contributor Author

@sba1 pinging you because I do not know if you get notified on forced pushes.

@sba1 sba1 merged commit 500769f into sba1:master Oct 17, 2023
@sba1
Copy link
Owner

sba1 commented Oct 17, 2023

Thanks!

@3246251196 3246251196 deleted the clib4_math_defines branch October 30, 2023 20:40
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.

None yet

2 participants