diff mbox

[RFA] Fix various PPC build failures due to int-in-boolean-context code

Message ID AM4PR0701MB216293B1F5DF3041FD9BF759E4A70@AM4PR0701MB2162.eurprd07.prod.outlook.com
State New
Headers show

Commit Message

Bernd Edlinger Nov. 7, 2016, 3:15 p.m. UTC
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.


Hmm...

 From the warning's perspective it would look far less suspicious,
if we make this an unsigned shift op.

I looked at options.h and I think we could also use one bit more
if the shift was unsigned.

Furthermore there are macros TARGET_..._P which do not put
brackets around the macro parameter.

So how about this?

Cross-compiler for powerpc-eabi builds without warning.

Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Bernd.

Comments

Jeff Law Nov. 23, 2016, 9:54 p.m. UTC | #1
On 11/07/2016 08:15 AM, Bernd Edlinger wrote:
> 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.
>
> Hmm...
>
>  From the warning's perspective it would look far less suspicious,
> if we make this an unsigned shift op.
>
> I looked at options.h and I think we could also use one bit more
> if the shift was unsigned.
>
> Furthermore there are macros TARGET_..._P which do not put
> brackets around the macro parameter.
>
> So how about this?
>
> Cross-compiler for powerpc-eabi builds without warning.
>
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?
>
>
> Bernd.
>
>
> patch-options.diff
>
>
> 2016-11-07  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>
> 	* opth-gen.awk: Use unsigned shifts for bit masks.  Allow all bits
> 	to be used.  Add brackets around macro argument.
Looks good.  I went ahead and committed as I want to start another 
config-list.mk run today and I'd like to see all the ppc targets pass 
this time :-0

jeff
diff mbox

Patch

2016-11-07  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* opth-gen.awk: Use unsigned shifts for bit masks.  Allow all bits
	to be used.  Add brackets around macro argument.

Index: gcc/opth-gen.awk
===================================================================
--- gcc/opth-gen.awk	(revision 241884)
+++ gcc/opth-gen.awk	(working copy)
@@ -350,11 +350,11 @@  for (i = 0; i < n_opts; i++) {
 		mask_bits[name] = 1
 		vname = var_name(flags[i])
 		mask = "MASK_"
-		mask_1 = "1"
+		mask_1 = "1U"
 		if (vname != "") {
 			mask = "OPTION_MASK_"
 			if (host_wide_int[vname] == "yes")
-				mask_1 = "HOST_WIDE_INT_1"
+				mask_1 = "HOST_WIDE_INT_1U"
 		} else
 			extra_mask_bits[name] = 1
 		print "#define " mask name " (" mask_1 " << " masknum[vname]++ ")"
@@ -362,16 +362,16 @@  for (i = 0; i < n_opts; i++) {
 }
 for (i = 0; i < n_extra_masks; i++) {
 	if (extra_mask_bits[extra_masks[i]] == 0)
-		print "#define MASK_" extra_masks[i] " (1 << " masknum[""]++ ")"
+		print "#define MASK_" extra_masks[i] " (1U << " masknum[""]++ ")"
 }
 
 for (var in masknum) {
 	if (var != "" && host_wide_int[var] == "yes") {
-		print" #if defined(HOST_BITS_PER_WIDE_INT) && " masknum[var] " >= HOST_BITS_PER_WIDE_INT"
+		print "#if defined(HOST_BITS_PER_WIDE_INT) && " masknum[var] " > HOST_BITS_PER_WIDE_INT"
 		print "#error too many masks for " var
 		print "#endif"
 	}
-	else if (masknum[var] > 31) {
+	else if (masknum[var] > 32) {
 		if (var == "")
 			print "#error too many target masks"
 		else
@@ -401,7 +401,7 @@  for (i = 0; i < n_opts; i++) {
 		print "#define TARGET_" name \
 		      " ((" vname " & " mask name ") != 0)"
 		print "#define TARGET_" name "_P(" vname ")" \
-		      " ((" vname " & " mask name ") != 0)"
+		      " (((" vname ") & " mask name ") != 0)"
 	}
 }
 for (i = 0; i < n_extra_masks; i++) {