[PATCH] gcc/configure.ac: fix --enable-fixed-point enablement [PR34422]

[PATCH] gcc/configure.ac: fix --enable-fixed-point enablement [PR34422]

[Eric Gallager]

[-- Attachment #1: Type: text/plain, Size: 562 bytes --]

So, in investigating PR target/34422, I discovered that the gcc
subdirectory's configure script had an instance of AC_ARG_ENABLE with
3rd and 4th its arguments reversed: the one where it warns that the
--enable-fixed-point flag is being ignored is the one where that flag
hasn't even been passed in the first place. The attached patch puts
the warning in the correct argument to the macro in question. (I'm not
including the regeneration of gcc/configure in the patch this time
since that confused people last time.) OK to commit, with an
appropriate ChangeLog?

[-- Attachment #2: patch-gcc_configure.diff --]
[-- Type: application/octet-stream, Size: 985 bytes --]

diff --git a/gcc/configure.ac b/gcc/configure.ac
index 80bdd8ceef9..5661d2382d7 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -941,7 +941,17 @@ point format instead of DPD])
 AC_ARG_ENABLE(fixed-point,
 [AS_HELP_STRING([--enable-fixed-point],
 		[enable fixed-point arithmetic extension to C])],
-[],
+[
+  case $target in
+    arm*-*-* | mips*-*-* | loongarch*-*-*)
+      # (enable_fixed_point already gets set to "yes" by the flag being passed)
+      ;;
+    *)
+      AC_MSG_WARN([fixed-point is not supported for this target, ignoring the --enable-fixed-point flag])
+      enable_fixed_point=no
+      ;;
+  esac
+],
 [
   case $target in
     arm*)
@@ -955,7 +965,8 @@ AC_ARG_ENABLE(fixed-point,
       enable_fixed_point=yes
       ;;
     *)
-      AC_MSG_WARN([fixed-point is not supported for this target, ignored])
+      # (no need to warn in this case, because no flag was given, so nothing was
+      # ignored)
       enable_fixed_point=no
       ;;
   esac

Re: [PATCH] gcc/configure.ac: fix --enable-fixed-point enablement [PR34422]

[Eric Gallager]

Hi, I'd like to ping this patch:
https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596654.html
(cc-ing the build machinery maintainers listed in MAINTAINERS this time)

On Tue, Jun 14, 2022 at 3:51 PM Eric Gallager <egall@gwmail.gwu.edu> wrote:
>
> So, in investigating PR target/34422, I discovered that the gcc
> subdirectory's configure script had an instance of AC_ARG_ENABLE with
> 3rd and 4th its arguments reversed: the one where it warns that the
> --enable-fixed-point flag is being ignored is the one where that flag
> hasn't even been passed in the first place. The attached patch puts
> the warning in the correct argument to the macro in question. (I'm not
> including the regeneration of gcc/configure in the patch this time
> since that confused people last time.) OK to commit, with an
> appropriate ChangeLog?

Re: [PATCH] gcc/configure.ac: fix --enable-fixed-point enablement [PR34422]

[Alexandre Oliva]

Hello, Eric,

On Jun 21, 2022, Eric Gallager <egall@gwmail.gwu.edu> wrote:

> https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596654.html
> (cc-ing the build machinery maintainers listed in MAINTAINERS this time)

Thanks

> On Tue, Jun 14, 2022 at 3:51 PM Eric Gallager <egall@gwmail.gwu.edu> wrote:

>> So, in investigating PR target/34422, I discovered that the gcc
>> subdirectory's configure script had an instance of AC_ARG_ENABLE with
>> 3rd and 4th its arguments reversed: the one where it warns that the
>> --enable-fixed-point flag is being ignored is the one where that flag
>> hasn't even been passed in the first place.

I'm not sure it was reversed or meant to enable the feature by default
(if not given), but neither theory holds much water to me.

Alas, the proposed change needs a little more work.  Specifically, I
don't think we should warn that the feature is not available if it was
explicitly --disable'd.

Moreover, if it was intended for fixed-point to be enabled by default,
then the help string should probably mention the --disable flag instead,
otherwise, the per-target setting to yes should probably be dropped.


I dislike the duplication of the targets in two case statements.  I
think the best way to deal with this would be to only set
enable_fixed_point=default in ACTION-IF-NOT-GIVEN, leaving
ACTION-IF-GIVEN empty, and afterwards, if "x$enable_fixed_point" !=
"xno", check for target support, bumping default to yes on supported
targets, and decaying it to no if unsupported, with a warning only if it
was yes.

>> with an appropriate ChangeLog?

It's good practice to post the proposed ChangeLog entry along with the
patch, so that it can also be reviewed.

Thanks,

-- 
Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
   Free Software Activist                       GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>