Drop -Wswitch-bool warning in function.c

Drop -Wswitch-bool warning in function.c

[Kito Cheng]

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

Bootstrapped & regression-tested on x86_64-linux-gnu :)

2015-07-08  Kito Cheng  <kito.cheng@gmail.com>

        * function.c (stack_protect_epilogue): Use if rather than switch for
        check targetm.have_stack_protect_test().

[-- Attachment #2: 0001-Drop-Wswitch-bool-warning-in-function.c.patch --]
[-- Type: text/x-patch, Size: 1488 bytes --]

From 0306990aac578167872a80ab55085d335e2bea14 Mon Sep 17 00:00:00 2001
From: Kito Cheng <kito@andestech.com>
Date: Wed, 8 Jul 2015 15:20:01 +0800
Subject: [PATCH] Drop -Wswitch-bool warning in function.c

---
 gcc/function.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/gcc/function.c b/gcc/function.c
index 972cdc8..b87aef6 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -4874,26 +4874,18 @@ stack_protect_epilogue (void)
   tree guard_decl = targetm.stack_protect_guard ();
   rtx_code_label *label = gen_label_rtx ();
   rtx x, y, tmp;
+  rtx_insn *seq;
 
   x = expand_normal (crtl->stack_protect_guard);
   y = expand_normal (guard_decl);
 
   /* Allow the target to compare Y with X without leaking either into
      a register.  */
-  switch (targetm.have_stack_protect_test ())
-    {
-    case 1:
-      if (rtx_insn *seq = targetm.gen_stack_protect_test (x, y, label))
-	{
-	  emit_insn (seq);
-	  break;
-	}
-      /* FALLTHRU */
-
-    default:
-      emit_cmp_and_jump_insns (x, y, EQ, NULL_RTX, ptr_mode, 1, label);
-      break;
-    }
+  if (targetm.have_stack_protect_test ()
+      && ((seq = targetm.gen_stack_protect_test (x, y, label)) != NULL_RTX))
+    emit_insn (seq);
+  else
+    emit_cmp_and_jump_insns (x, y, EQ, NULL_RTX, ptr_mode, 1, label);
 
   /* The noreturn predictor has been moved to the tree level.  The rtl-level
      predictors estimate this branch about 20%, which isn't enough to get
-- 
2.3.5

Re: Drop -Wswitch-bool warning in function.c

[Marek Polacek]

On Wed, Jul 08, 2015 at 04:49:19PM +0800, Kito Cheng wrote:
> Bootstrapped & regression-tested on x86_64-linux-gnu :)
> 
> 2015-07-08  Kito Cheng  <kito.cheng@gmail.com>
> 
>         * function.c (stack_protect_epilogue): Use if rather than switch for
>         check targetm.have_stack_protect_test().

Do you really need this?  This should be just non-fatal warning in stage1,
and only if you compile with gcc-5, right?
You could also just cast the controlling expression of the switch to int to
suppress the warning.

	Marek

Re: Drop -Wswitch-bool warning in function.c

[Kito Cheng]

Hi Marek:

Yes, I know it's non-fatal warning, but I think gcc should build with
--enable-werror-always by it's self
and it's the *ONLY* warning in trunk now.

Of cause, cast to int can suppress the warning,
but it's not good solution so gcc complain that switch condition has
bool type :)

On Wed, Jul 8, 2015 at 9:30 PM, Marek Polacek <polacek@redhat.com> wrote:
> On Wed, Jul 08, 2015 at 04:49:19PM +0800, Kito Cheng wrote:
>> Bootstrapped & regression-tested on x86_64-linux-gnu :)
>>
>> 2015-07-08  Kito Cheng  <kito.cheng@gmail.com>
>>
>>         * function.c (stack_protect_epilogue): Use if rather than switch for
>>         check targetm.have_stack_protect_test().
>
> Do you really need this?  This should be just non-fatal warning in stage1,
> and only if you compile with gcc-5, right?
> You could also just cast the controlling expression of the switch to int to
> suppress the warning.
>
>         Marek

Re: Drop -Wswitch-bool warning in function.c

[Jeff Law]

On 07/08/2015 02:49 AM, Kito Cheng wrote:
> Bootstrapped & regression-tested on x86_64-linux-gnu :)
>
> 2015-07-08  Kito Cheng  <kito.cheng@gmail.com>
>
>          * function.c (stack_protect_epilogue): Use if rather than switch for
>          check targetm.have_stack_protect_test().
>
OK.  Not necessarily because avoid the warning is really all that 
important, but because the if-else is just as easy to read as the switch 
and dramatically more compact.

Jeff

Re: Drop -Wswitch-bool warning in function.c

[Kito Cheng]

Hi Jeff:
Thanks your review and approve, however I don't have commit right yet,
 can you help me to commit it :)

thanks

On Thu, Jul 9, 2015 at 5:00 AM, Jeff Law <law@redhat.com> wrote:
> On 07/08/2015 02:49 AM, Kito Cheng wrote:
>>
>> Bootstrapped & regression-tested on x86_64-linux-gnu :)
>>
>> 2015-07-08  Kito Cheng  <kito.cheng@gmail.com>
>>
>>          * function.c (stack_protect_epilogue): Use if rather than switch
>> for
>>          check targetm.have_stack_protect_test().
>>
> OK.  Not necessarily because avoid the warning is really all that important,
> but because the if-else is just as easy to read as the switch and
> dramatically more compact.
>
> Jeff

Re: Drop -Wswitch-bool warning in function.c

[Jeff Law]

On 07/08/2015 09:25 PM, Kito Cheng wrote:
> Hi Jeff:
> Thanks your review and approve, however I don't have commit right yet,
>   can you help me to commit it :)
Committed to the trunk.
jeff

Re: Drop -Wswitch-bool warning in function.c

[Richard Sandiford]

Kito Cheng <kito.cheng@gmail.com> writes:
> Yes, I know it's non-fatal warning, but I think gcc should build with
> --enable-werror-always by it's self
> and it's the *ONLY* warning in trunk now.

Yeah, but it should only build with --enable-werror-always if your host
compiler is the same version as the one you're building.  It would be too
much to expect it to compile cleanly with older versions too, since there's
no obvious place to draw the line.

So Marek's point was that current GCC (deliberately) does not warn about
this switch statement.  The switch statement used to be:

  /* Allow the target to compare Y with X without leaking either into
     a register.  */
  switch ((int) (HAVE_stack_protect_test != 0))

but the (int) cast was removed at the same time as the warning was
adjusted:

  https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00790.html

(BTW, although you cc:ed me, it was that patch rather than mine that
introduced the warnings when using unpatched host compilers.)

Thanks,
Richard