RFA: Fix bootstrap/46358

RFA: Fix bootstrap/46358

[Joern Rennecke]

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

Currently bootstrapping on i686-pc-linux-gnu.

[-- Attachment #2: pr46358-patch --]
[-- Type: text/plain, Size: 1252 bytes --]

2010-11-08  Joern Rennecke  <amylaar@spamcop.net>

	PR bootstrap/46358
	* config/i386/i386.c (ix86_expand_split_stack_prologue):
	Avoid warnings when HOST_WIDE_INT is 32 bit.

Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 166429)
+++ config/i386/i386.c	(working copy)
@@ -11171,8 +11171,8 @@ ix86_expand_split_stack_prologue (void)
 	     different function: __morestack_large.  We pass the
 	     argument size in the upper 32 bits of r10 and pass the
 	     frame size in the lower 32 bits.  */
-	  gcc_assert ((allocate & 0xffffffff) == allocate);
-	  gcc_assert (((HOST_WIDE_INT) args_size & 0xffffffff)
+	  gcc_assert ((allocate & (HOST_WIDE_INT) 0xffffffff) == allocate);
+	  gcc_assert (((HOST_WIDE_INT) args_size & (HOST_WIDE_INT) 0xffffffff)
 		      == (HOST_WIDE_INT) args_size);
 
 	  if (split_stack_fn_large == NULL_RTX)
@@ -11202,7 +11202,8 @@ ix86_expand_split_stack_prologue (void)
 
 	  fn = reg11;
 
-	  argval = ((HOST_WIDE_INT) args_size << 32) + allocate;
+	  argval = (HOST_BITS_PER_WIDE_INT > 32
+		    ? (HOST_WIDE_INT) args_size << 32 : 0) + allocate;
 	  emit_move_insn (reg10, GEN_INT (argval));
 	}
       else

Re: RFA: Fix bootstrap/46358

[Richard Henderson]

On 11/08/2010 01:57 AM, Joern Rennecke wrote:
> +	  gcc_assert ((allocate & (HOST_WIDE_INT) 0xffffffff) == allocate);
> +	  gcc_assert (((HOST_WIDE_INT) args_size & (HOST_WIDE_INT) 0xffffffff)
>  		      == (HOST_WIDE_INT) args_size);

Please just promote args_size to HOST_WIDE_INT.

> +	  argval = (HOST_BITS_PER_WIDE_INT > 32
> +		    ? (HOST_WIDE_INT) args_size << 32 : 0) + allocate;

Cleaner to split to the shift in two.

Ok with those changes.


r~

Re: RFA: Fix bootstrap/46358

[Joern Rennecke]

Quoting Richard Henderson <rth@redhat.com>:

> On 11/08/2010 01:57 AM, Joern Rennecke wrote:
>> +	  gcc_assert ((allocate & (HOST_WIDE_INT) 0xffffffff) == allocate);
>> +	  gcc_assert (((HOST_WIDE_INT) args_size & (HOST_WIDE_INT) 0xffffffff)
>>  		      == (HOST_WIDE_INT) args_size);
>
> Please just promote args_size to HOST_WIDE_INT.

It already is - this has no effect because HOST_WIDE_INT is just a 32  
bit long.
Or do you mean unsigned HOST_WIDE_INT?

>
>> +	  argval = (HOST_BITS_PER_WIDE_INT > 32
>> +		    ? (HOST_WIDE_INT) args_size << 32 : 0) + allocate;
>
> Cleaner to split to the shift in two.

Like this?
argval = ((HOST_WIDE_INT) args_size << 16) << 16) + allocate;

Re: RFA: Fix bootstrap/46358

[Richard Henderson]

On 11/08/2010 09:11 AM, Joern Rennecke wrote:
> Quoting Richard Henderson <rth@redhat.com>:
> 
>> On 11/08/2010 01:57 AM, Joern Rennecke wrote:
>>> +      gcc_assert ((allocate & (HOST_WIDE_INT) 0xffffffff) == allocate);
>>> +      gcc_assert (((HOST_WIDE_INT) args_size & (HOST_WIDE_INT) 0xffffffff)
>>>                == (HOST_WIDE_INT) args_size);
>>
>> Please just promote args_size to HOST_WIDE_INT.
> 
> It already is - this has no effect because HOST_WIDE_INT is just a 32 bit long.

I don't see that in my tree.

ix86_expand_split_stack_prologue (void)
{
  struct ix86_frame frame;
  HOST_WIDE_INT allocate;
  int args_size;

  ^^^

> Like this?
> argval = ((HOST_WIDE_INT) args_size << 16) << 16) + allocate;

Yes, thanks.


r~

Re: RFA: Fix bootstrap/46358

[Ian Lance Taylor]

Joern Rennecke <amylaar@spamcop.net> writes:

> Currently bootstrapping on i686-pc-linux-gnu.
> 2010-11-08  Joern Rennecke  <amylaar@spamcop.net>
>
> 	PR bootstrap/46358
> 	* config/i386/i386.c (ix86_expand_split_stack_prologue):
> 	Avoid warnings when HOST_WIDE_INT is 32 bit.

Thanks, sorry about that.

Ian

Re: RFA: Fix bootstrap/46358

[amylaar]

Quoting Richard Henderson <rth@redhat.com>:

> I don't see that in my tree.

I mean the current version in svn already casts it to HOST_WIDE_INT.

Re: RFA: Fix bootstrap/46358

[Joern Rennecke]

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

This is the amended patch.

Bootstrapped on i686-pc-linux-gnu.

[-- Attachment #2: pr46358-patch-2 --]
[-- Type: text/plain, Size: 1291 bytes --]

2010-11-08  Joern Rennecke  <amylaar@spamcop.net>

	PR bootstrap/46358
	* config/i386/i386.c (ix86_expand_split_stack_prologue):
	Avoid warnings when HOST_WIDE_INT is 32 bit.

Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 166429)
+++ config/i386/i386.c	(working copy)
@@ -11171,9 +11171,9 @@ ix86_expand_split_stack_prologue (void)
 	     different function: __morestack_large.  We pass the
 	     argument size in the upper 32 bits of r10 and pass the
 	     frame size in the lower 32 bits.  */
-	  gcc_assert ((allocate & 0xffffffff) == allocate);
-	  gcc_assert (((HOST_WIDE_INT) args_size & 0xffffffff)
-		      == (HOST_WIDE_INT) args_size);
+	  gcc_assert ((allocate & (HOST_WIDE_INT) 0xffffffff) == allocate);
+	  gcc_assert (((unsigned HOST_WIDE_INT) args_size & 0xffffffff)
+		      == (unsigned HOST_WIDE_INT) args_size);
 
 	  if (split_stack_fn_large == NULL_RTX)
 	    split_stack_fn_large =
@@ -11202,7 +11202,7 @@ ix86_expand_split_stack_prologue (void)
 
 	  fn = reg11;
 
-	  argval = ((HOST_WIDE_INT) args_size << 32) + allocate;
+	  argval = (((HOST_WIDE_INT) args_size << 16) << 16) + allocate;
 	  emit_move_insn (reg10, GEN_INT (argval));
 	}
       else

Re: RFA: Fix bootstrap/46358

[Richard Henderson]

On 11/08/2010 09:23 AM, amylaar@spamcop.net wrote:
> Quoting Richard Henderson <rth@redhat.com>:
> 
>> I don't see that in my tree.
> 
> I mean the current version in svn already casts it to HOST_WIDE_INT.

I mean that you should change the type of the variable instead
of casting it nearly every place it gets used.


r~

Re: RFA: Fix bootstrap/46358

[Ian Lance Taylor]

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

amylaar@spamcop.net writes:

> Quoting Richard Henderson <rth@redhat.com>:
>
>> I don't see that in my tree.
>
> I mean the current version in svn already casts it to HOST_WIDE_INT.

I think Richard meant something more like this (this patch is against
current trunk).

Ian


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: HOST_WIDE_INT --]
[-- Type: text/x-diff, Size: 1248 bytes --]

Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 166445)
+++ config/i386/i386.c	(working copy)
@@ -11063,7 +11063,7 @@ ix86_expand_split_stack_prologue (void)
 {
   struct ix86_frame frame;
   HOST_WIDE_INT allocate;
-  int args_size;
+  HOST_WIDE_INT args_size;
   rtx label, limit, current, jump_insn, allocate_rtx, call_insn, call_fusage;
   rtx scratch_reg = NULL_RTX;
   rtx varargs_label = NULL_RTX;
@@ -11172,8 +11172,7 @@ ix86_expand_split_stack_prologue (void)
 	     argument size in the upper 32 bits of r10 and pass the
 	     frame size in the lower 32 bits.  */
 	  gcc_assert ((allocate & (HOST_WIDE_INT) 0xffffffff) == allocate);
-	  gcc_assert (((unsigned HOST_WIDE_INT) args_size & 0xffffffff)
-		      == (unsigned HOST_WIDE_INT) args_size);
+	  gcc_assert ((args_size & (HOST_WIDE_INT) 0xffffffff) == args_size);
 
 	  if (split_stack_fn_large == NULL_RTX)
 	    split_stack_fn_large =
@@ -11202,7 +11201,7 @@ ix86_expand_split_stack_prologue (void)
 
 	  fn = reg11;
 
-	  argval = (((HOST_WIDE_INT) args_size << 16) << 16) + allocate;
+	  argval = ((args_size << 16) << 16) + allocate;
 	  emit_move_insn (reg10, GEN_INT (argval));
 	}
       else

Re: RFA: Fix bootstrap/46358

[Joern Rennecke]

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

Quoting Ian Lance Taylor <iant@google.com>:

> I think Richard meant something more like this (this patch is against
> current trunk).

But that would still leave the variable with a type that is not suitable
for all contexts.  How about this patch instead?

[-- Attachment #2: i386-arg_size-uhwint --]
[-- Type: text/plain, Size: 1345 bytes --]

2010-11-09  Joern Rennecke  <amylaar@spamcop.net>

	* ix86_expand_split_stack_prologue (args_size): Change type to
	unsigned HOST_WIDE_INT.

--- i386.c	2010-11-08 17:56:11.693608704 +0000
+++ i386.c-uhwint	2010-11-09 08:51:18.265358933 +0000
@@ -11063,7 +11063,7 @@ ix86_expand_split_stack_prologue (void)
 {
   struct ix86_frame frame;
   HOST_WIDE_INT allocate;
-  int args_size;
+  unsigned HOST_WIDE_INT args_size;
   rtx label, limit, current, jump_insn, allocate_rtx, call_insn, call_fusage;
   rtx scratch_reg = NULL_RTX;
   rtx varargs_label = NULL_RTX;
@@ -11172,8 +11172,7 @@ ix86_expand_split_stack_prologue (void)
 	     argument size in the upper 32 bits of r10 and pass the
 	     frame size in the lower 32 bits.  */
 	  gcc_assert ((allocate & (HOST_WIDE_INT) 0xffffffff) == allocate);
-	  gcc_assert (((unsigned HOST_WIDE_INT) args_size & 0xffffffff)
-		      == (unsigned HOST_WIDE_INT) args_size);
+	  gcc_assert ((args_size & 0xffffffff) == args_size);
 
 	  if (split_stack_fn_large == NULL_RTX)
 	    split_stack_fn_large =
@@ -11202,7 +11201,7 @@ ix86_expand_split_stack_prologue (void)
 
 	  fn = reg11;
 
-	  argval = (((HOST_WIDE_INT) args_size << 16) << 16) + allocate;
+	  argval = ((args_size << 16) << 16) + allocate;
 	  emit_move_insn (reg10, GEN_INT (argval));
 	}
       else

Re: RFA: Fix bootstrap/46358

[Ian Lance Taylor]

Joern Rennecke <amylaar@spamcop.net> writes:

> 2010-11-09  Joern Rennecke  <amylaar@spamcop.net>
>
> 	* ix86_expand_split_stack_prologue (args_size): Change type to
> 	unsigned HOST_WIDE_INT.

This is OK if it bootstraps.

Thanks.

Ian