| Message ID | f746cfca-4b85-cf1e-bb57-229845e64e25@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Oct 28, 2016 at 09:12:29AM -0600, Jeff Law wrote: > * config/rs6000/rs6000.c (rs6000_option_override_internal): Avoid > false positive from int-in-boolean-context warnings. > > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c > index 5e35e33..38a5226 100644 > --- a/gcc/config/rs6000/rs6000.c > +++ b/gcc/config/rs6000/rs6000.c > @@ -3880,7 +3880,7 @@ rs6000_option_override_internal (bool global_init_p) > > If there is a TARGET_DEFAULT, use that. Otherwise fall back to using > -mcpu=powerpc, -mcpu=powerpc64, or -mcpu=powerpc64le defaults. */ > - HOST_WIDE_INT flags = ((TARGET_DEFAULT) ? TARGET_DEFAULT > + HOST_WIDE_INT flags = ((TARGET_DEFAULT) != 0 ? TARGET_DEFAULT Why ()s around TARGET_DEFAULT? If they are needed, they should be provided in the TARGET_DEFAULT macro definition. So I think HOST_WIDE_INT flags = (TARGET_DEFAULT != 0 ? TARGET_DEFAULT : processor_target_table[cpu_index].target_enable); is what we want to use (the processor_target_table[cpu_index].target_enable line is too long where it is right now). > : processor_target_table[cpu_index].target_enable); > rs6000_isa_flags |= (flags & ~rs6000_isa_flags_explicit); > } Jakub
On 10/28/2016 09:17 AM, Jakub Jelinek wrote: > On Fri, Oct 28, 2016 at 09:12:29AM -0600, Jeff Law wrote: >> * config/rs6000/rs6000.c (rs6000_option_override_internal): Avoid >> false positive from int-in-boolean-context warnings. >> >> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c >> index 5e35e33..38a5226 100644 >> --- a/gcc/config/rs6000/rs6000.c >> +++ b/gcc/config/rs6000/rs6000.c >> @@ -3880,7 +3880,7 @@ rs6000_option_override_internal (bool global_init_p) >> >> If there is a TARGET_DEFAULT, use that. Otherwise fall back to using >> -mcpu=powerpc, -mcpu=powerpc64, or -mcpu=powerpc64le defaults. */ >> - HOST_WIDE_INT flags = ((TARGET_DEFAULT) ? TARGET_DEFAULT >> + HOST_WIDE_INT flags = ((TARGET_DEFAULT) != 0 ? TARGET_DEFAULT > > Why ()s around TARGET_DEFAULT? Not strictly needed. But I didn't see a need to change that given they've been in the port "forever" and they don't impact readability in any significant way. If they are needed, they should be provided > in the TARGET_DEFAULT macro definition. So I think > HOST_WIDE_INT flags > = (TARGET_DEFAULT != 0 ? TARGET_DEFAULT > : processor_target_table[cpu_index].target_enable); > is what we want to use (the processor_target_table[cpu_index].target_enable > line is too long where it is right now). Agreed. I wasn't looking to do any cleanups, just get everything building again with config-list.mk. jeff
On Fri, Oct 28, 2016 at 09:12:29AM -0600, Jeff Law wrote: > > The PPC port is stumbling over the new integer in boolean context warnings. > > In particular this code from rs6000_option_override_internal is > problematical: > > HOST_WIDE_INT flags = ((TARGET_DEFAULT) ? TARGET_DEFAULT > : > processor_target_table[cpu_index].target_enable); > > The compiler is flagging the (TARGET_DEFAULT) condition. That's > supposed to to be a boolean. > > After all the macro expansions are done it ultimately looks something > like this: > > long flags = (((1L << 7)) ? (1L << 7) > : processor_target_table[cpu_index].target_enable); > > Note the (1L << 7) used as the condition for the ternary. That's what > has the int-in-boolean-context warning tripping. It's a false positive > IMHO. Yes, there is an implicit conversion to bool here afaics. So can the misfiring warning be fixed instead, please? > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c > index 5e35e33..38a5226 100644 > --- a/gcc/config/rs6000/rs6000.c > +++ b/gcc/config/rs6000/rs6000.c > @@ -3880,7 +3880,7 @@ rs6000_option_override_internal (bool global_init_p) > > If there is a TARGET_DEFAULT, use that. Otherwise fall back to using > -mcpu=powerpc, -mcpu=powerpc64, or -mcpu=powerpc64le defaults. */ > - HOST_WIDE_INT flags = ((TARGET_DEFAULT) ? TARGET_DEFAULT > + HOST_WIDE_INT flags = ((TARGET_DEFAULT) != 0 ? TARGET_DEFAULT > : processor_target_table[cpu_index].target_enable); > rs6000_isa_flags |= (flags & ~rs6000_isa_flags_explicit); > } Eww. HOST_WIDE_INT flags = TARGET_DEFAULT; if (flags == 0) flags = processor_target_table[cpu_index].target_enable); instead, if you have to do anything? Segher
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index 5e35e33..38a5226 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -3880,7 +3880,7 @@ rs6000_option_override_internal (bool global_init_p) If there is a TARGET_DEFAULT, use that. Otherwise fall back to using -mcpu=powerpc, -mcpu=powerpc64, or -mcpu=powerpc64le defaults. */ - HOST_WIDE_INT flags = ((TARGET_DEFAULT) ? TARGET_DEFAULT + HOST_WIDE_INT flags = ((TARGET_DEFAULT) != 0 ? TARGET_DEFAULT : processor_target_table[cpu_index].target_enable); rs6000_isa_flags |= (flags & ~rs6000_isa_flags_explicit); }
The PPC port is stumbling over the new integer in boolean context warnings. In particular this code from rs6000_option_override_internal is problematical: HOST_WIDE_INT flags = ((TARGET_DEFAULT) ? TARGET_DEFAULT : processor_target_table[cpu_index].target_enable); The compiler is flagging the (TARGET_DEFAULT) condition. That's supposed to to be a boolean. After all the macro expansions are done it ultimately looks something like this: long flags = (((1L << 7)) ? (1L << 7) : processor_target_table[cpu_index].target_enable); Note the (1L << 7) used as the condition for the ternary. That's what has the int-in-boolean-context warning tripping. It's a false positive IMHO. Working around the warning is pretty trivial, we can just compare against zero. ie ((TARGET_DEFAULT) != 0 ? ... : ...; With that change all the PPC configurations in config-list.mk can be built with a trunk compiler. OK for the trunk? Jeff * config/rs6000/rs6000.c (rs6000_option_override_internal): Avoid false positive from int-in-boolean-context warnings.