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++) {