public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Emit more REG_EQUIV notes for function args (PR42235)
@ 2010-07-14 11:14 Bernd Schmidt
  2010-07-14 18:54 ` Jeff Law
  0 siblings, 1 reply; 15+ messages in thread
From: Bernd Schmidt @ 2010-07-14 11:14 UTC (permalink / raw)
  To: GCC Patches

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

When moving arguments into pseudos, we are being very careful not to
emit any instructions that could possibly clobber other argument
registers.  Currently, we generate unnecessarily complicated sequences
of code for simple zero or sign extensions.

With this patch, we check can_extend_p and the necessary predicates to
see if an extend insn is available for the conversion we have to do.  If
so, we emit the insn directly, and create a REG_EQUIV note of the form
(sign_extend (mem)).  Reload can't really do anything with these yet,
but there's an optimization in the register allocator to move argument
loads directly before their use if there's only one use.  On Thumb-2
this happens already, we seem to generate better code than before.
Unfortunately Thumb-1, which the PR is about, has some other problems
which I'll try to fix later.

Bootstrapped and regression tested on i686-linux.  Also regression
tested on arm-linux, with my three usual sets of options.  Ok?


Bernd

[-- Attachment #2: more-equiv-notes.diff --]
[-- Type: text/plain, Size: 3323 bytes --]

	PR target/42235
	* function.c (assign_parm_setup_reg): When an optab for extending
	exists, emit the insn directly and create a REG_EQUIV note.

Index: function.c
===================================================================
--- function.c	(revision 162146)
+++ function.c	(working copy)
@@ -2861,10 +2861,12 @@ static void
 assign_parm_setup_reg (struct assign_parm_data_all *all, tree parm,
 		       struct assign_parm_data_one *data)
 {
-  rtx parmreg;
+  rtx parmreg, validated_mem;
+  rtx equiv_stack_parm;
   enum machine_mode promoted_nominal_mode;
   int unsignedp = TYPE_UNSIGNED (TREE_TYPE (parm));
   bool did_conversion = false;
+  bool need_conversion, moved;
 
   /* Store the parm in a pseudoregister during the function, but we may
      need to do it in a wider mode.  Using 2 here makes the result
@@ -2891,10 +2893,46 @@ assign_parm_setup_reg (struct assign_par
 
   assign_parm_remove_parallels (data);
 
+  equiv_stack_parm = data->stack_parm;
+  validated_mem = validize_mem (data->entry_parm);
+
+  need_conversion = (data->nominal_mode != data->passed_mode
+		     || promoted_nominal_mode != data->promoted_mode);
+  moved = false;
+
   /* Copy the value into the register, thus bridging between
      assign_parm_find_data_types and expand_expr_real_1.  */
-  if (data->nominal_mode != data->passed_mode
-      || promoted_nominal_mode != data->promoted_mode)
+  if (need_conversion)
+    {
+      enum insn_code icode;
+      rtx op0, op1;
+
+      icode = can_extend_p (promoted_nominal_mode, data->passed_mode,
+			    unsignedp);
+
+      op0 = parmreg;
+      op1 = validated_mem;
+      if (icode != CODE_FOR_nothing
+	  && insn_data[icode].operand[0].predicate (op0, promoted_nominal_mode)
+	  && insn_data[icode].operand[1].predicate (op1, data->passed_mode))
+	{
+	  enum rtx_code code = unsignedp ? ZERO_EXTEND : SIGN_EXTEND;
+	  rtx insn;
+
+	  insn = gen_extend_insn (op0, op1, promoted_nominal_mode,
+				  data->passed_mode, unsignedp);
+	  emit_insn (insn);
+
+	  equiv_stack_parm = gen_rtx_fmt_e (code, GET_MODE (parmreg),
+					    equiv_stack_parm);
+	  moved = true;
+	}
+    }
+
+  if (moved)
+    /* Nothing to do.  */
+    ;
+  else if (need_conversion)
     {
       int save_tree_used;
 
@@ -2919,7 +2957,7 @@ assign_parm_setup_reg (struct assign_par
 
       rtx tempreg = gen_reg_rtx (GET_MODE (data->entry_parm));
 
-      emit_move_insn (tempreg, validize_mem (data->entry_parm));
+      emit_move_insn (tempreg, validated_mem);
 
       push_to_sequence2 (all->first_conversion_insn, all->last_conversion_insn);
       tempreg = convert_to_mode (data->nominal_mode, tempreg, unsignedp);
@@ -2949,7 +2987,7 @@ assign_parm_setup_reg (struct assign_par
       did_conversion = true;
     }
   else
-    emit_move_insn (parmreg, validize_mem (data->entry_parm));
+    emit_move_insn (parmreg, validated_mem);
 
   /* If we were passed a pointer but the actual value can safely live
      in a register, put it in one.  */
@@ -3034,7 +3072,7 @@ assign_parm_setup_reg (struct assign_par
 	}
       else if ((set = single_set (linsn)) != 0
 	       && SET_DEST (set) == parmreg)
-	set_unique_reg_note (linsn, REG_EQUIV, data->stack_parm);
+	set_unique_reg_note (linsn, REG_EQUIV, equiv_stack_parm);
     }
 
   /* For pointer data type, suggest pointer register.  */

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Emit more REG_EQUIV notes for function args (PR42235)
  2010-07-14 11:14 Emit more REG_EQUIV notes for function args (PR42235) Bernd Schmidt
@ 2010-07-14 18:54 ` Jeff Law
  2010-07-14 21:30   ` Bernd Schmidt
  2010-07-14 21:48   ` Bernd Schmidt
  0 siblings, 2 replies; 15+ messages in thread
From: Jeff Law @ 2010-07-14 18:54 UTC (permalink / raw)
  To: Bernd Schmidt; +Cc: GCC Patches

On 07/14/10 05:14, Bernd Schmidt wrote:
> When moving arguments into pseudos, we are being very careful not to
> emit any instructions that could possibly clobber other argument
> registers.  Currently, we generate unnecessarily complicated sequences
> of code for simple zero or sign extensions.
>
> With this patch, we check can_extend_p and the necessary predicates to
> see if an extend insn is available for the conversion we have to do.
Right, but what guarantee do we have that the conversion insn doesn't 
clobber a function argument register?   ISTM that to be safe you 
actually have to scan the insns created by gen_extend_insn to ensure 
they don't clobber something important.

I'm not an expert on what ports do these days, but I did work on a port 
(mn10200) where conversion "insns" where implemented as special function 
calls under the hood.  I don't recall if we allowed those special 
function calls to have visible side effects, but if they did, they'd 
show up as clobbers/uses attached to the normal conversion insn.    Of 
course the mn102 is dead, but I think it's method for implementing 
conversions was valid and if another port were to do something similar 
it would likely not interact well with your change.


>   If
> so, we emit the insn directly, and create a REG_EQUIV note of the form
> (sign_extend (mem)).
Creating more REG_EQUIVs seems like a good idea to me.



>    Reload can't really do anything with these yet,
> but there's an optimization in the register allocator to move argument
> loads directly before their use if there's only one use.  On Thumb-2
> this happens already, we seem to generate better code than before.
> Unfortunately Thumb-1, which the PR is about, has some other problems
> which I'll try to fix later.
>    
In theory, given the REG_EQUIV note we ought to get an entry in 
reg_equiv_mem, which the code I'm working on knows it can use instead of 
shoving the pseudo into a stack slot.  Effectively, my code will 
rematerialize the argument from the equivalent memory location 
regardless of the number of uses.


Jeff

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Emit more REG_EQUIV notes for function args (PR42235)
  2010-07-14 18:54 ` Jeff Law
@ 2010-07-14 21:30   ` Bernd Schmidt
  2010-07-14 22:28     ` Jeff Law
  2010-07-14 21:48   ` Bernd Schmidt
  1 sibling, 1 reply; 15+ messages in thread
From: Bernd Schmidt @ 2010-07-14 21:30 UTC (permalink / raw)
  To: Jeff Law; +Cc: GCC Patches

On 07/14/2010 08:54 PM, Jeff Law wrote:
> On 07/14/10 05:14, Bernd Schmidt wrote:
>> When moving arguments into pseudos, we are being very careful not to
>> emit any instructions that could possibly clobber other argument
>> registers.  Currently, we generate unnecessarily complicated sequences
>> of code for simple zero or sign extensions.
>>
>> With this patch, we check can_extend_p and the necessary predicates to
>> see if an extend insn is available for the conversion we have to do.
> Right, but what guarantee do we have that the conversion insn doesn't
> clobber a function argument register?   ISTM that to be safe you
> actually have to scan the insns created by gen_extend_insn to ensure
> they don't clobber something important.
> 
> I'm not an expert on what ports do these days, but I did work on a port
> (mn10200) where conversion "insns" where implemented as special function
> calls under the hood.  I don't recall if we allowed those special
> function calls to have visible side effects, but if they did, they'd
> show up as clobbers/uses attached to the normal conversion insn.    Of
> course the mn102 is dead, but I think it's method for implementing
> conversions was valid and if another port were to do something similar
> it would likely not interact well with your change.

Hmm, ok.  That's awful, but I kind of expected someone would say that.
Did this really happen for integer zero/sign extend, or only for
floating point stuff?

If necessary I can try to test for a single insn with single_set and
push it to the sequence otherwise.


Bernd

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Emit more REG_EQUIV notes for function args (PR42235)
  2010-07-14 18:54 ` Jeff Law
  2010-07-14 21:30   ` Bernd Schmidt
@ 2010-07-14 21:48   ` Bernd Schmidt
  2010-07-14 22:22     ` Jeff Law
  1 sibling, 1 reply; 15+ messages in thread
From: Bernd Schmidt @ 2010-07-14 21:48 UTC (permalink / raw)
  To: Jeff Law; +Cc: GCC Patches

On 07/14/2010 08:54 PM, Jeff Law wrote:
> In theory, given the REG_EQUIV note we ought to get an entry in
> reg_equiv_mem, which the code I'm working on knows it can use instead of
> shoving the pseudo into a stack slot.  Effectively, my code will
> rematerialize the argument from the equivalent memory location
> regardless of the number of uses.

Reload can do that already, I think, but it's not prepared to handle a
(zero_extend (mem)) inside a REG_EQUIV note.  That should be reasonably
trivial to add if the reg is never set other than in the initializing insn.


Bernd

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Emit more REG_EQUIV notes for function args (PR42235)
  2010-07-14 21:48   ` Bernd Schmidt
@ 2010-07-14 22:22     ` Jeff Law
  0 siblings, 0 replies; 15+ messages in thread
From: Jeff Law @ 2010-07-14 22:22 UTC (permalink / raw)
  To: Bernd Schmidt; +Cc: GCC Patches

On 07/14/10 15:47, Bernd Schmidt wrote:
> On 07/14/2010 08:54 PM, Jeff Law wrote:
>    
>> In theory, given the REG_EQUIV note we ought to get an entry in
>> reg_equiv_mem, which the code I'm working on knows it can use instead of
>> shoving the pseudo into a stack slot.  Effectively, my code will
>> rematerialize the argument from the equivalent memory location
>> regardless of the number of uses.
>>      
> Reload can do that already, I think, but it's not prepared to handle a
> (zero_extend (mem)) inside a REG_EQUIV note.  That should be reasonably
> trivial to add if the reg is never set other than in the initializing insn.
>    
reload does, but it's code that can be problematical.  For example, 
replacement of an unallocated pseudo with its equivalent can trigger 
secondary reloads, spills, etc.

My code creates a new pseudo/allocno for the various ranges where the 
unallocated pseudo is live then asks IRA to color those split-range 
pseudos/allocnos.  What we end up presenting to reload is cleaner; 
though often they result in the same final code.  Handling this case 
falls out nicely from the infrastructure for splitting ranges of 
unallocated pseudos without equivalent forms (it's probably a couple 
dozen lines of new code).

jeff

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Emit more REG_EQUIV notes for function args (PR42235)
  2010-07-14 21:30   ` Bernd Schmidt
@ 2010-07-14 22:28     ` Jeff Law
  2010-07-15 17:14       ` Bernd Schmidt
  0 siblings, 1 reply; 15+ messages in thread
From: Jeff Law @ 2010-07-14 22:28 UTC (permalink / raw)
  To: Bernd Schmidt; +Cc: GCC Patches

On 07/14/10 15:30, Bernd Schmidt wrote:
> On 07/14/2010 08:54 PM, Jeff Law wrote:
>    
>> On 07/14/10 05:14, Bernd Schmidt wrote:
>>      
>>> When moving arguments into pseudos, we are being very careful not to
>>> emit any instructions that could possibly clobber other argument
>>> registers.  Currently, we generate unnecessarily complicated sequences
>>> of code for simple zero or sign extensions.
>>>
>>> With this patch, we check can_extend_p and the necessary predicates to
>>> see if an extend insn is available for the conversion we have to do.
>>>        
>> Right, but what guarantee do we have that the conversion insn doesn't
>> clobber a function argument register?   ISTM that to be safe you
>> actually have to scan the insns created by gen_extend_insn to ensure
>> they don't clobber something important.
>>
>> I'm not an expert on what ports do these days, but I did work on a port
>> (mn10200) where conversion "insns" where implemented as special function
>> calls under the hood.  I don't recall if we allowed those special
>> function calls to have visible side effects, but if they did, they'd
>> show up as clobbers/uses attached to the normal conversion insn.    Of
>> course the mn102 is dead, but I think it's method for implementing
>> conversions was valid and if another port were to do something similar
>> it would likely not interact well with your change.
>>      
> Hmm, ok.  That's awful, but I kind of expected someone would say that.
> Did this really happen for integer zero/sign extend, or only for
> floating point stuff?
>    
It was all the PSImode crap.

> If necessary I can try to test for a single insn with single_set and
> push it to the sequence otherwise.
>    
For the mn103, the conversions were single insns...

Ultimately,  I think you have to peek at the insn(s) and see what 
registers they set/clobber.

jeff


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Emit more REG_EQUIV notes for function args (PR42235)
  2010-07-14 22:28     ` Jeff Law
@ 2010-07-15 17:14       ` Bernd Schmidt
  2010-07-15 21:26         ` Jeff Law
  2010-07-19  9:55         ` Bernd Schmidt
  0 siblings, 2 replies; 15+ messages in thread
From: Bernd Schmidt @ 2010-07-15 17:14 UTC (permalink / raw)
  To: Jeff Law; +Cc: GCC Patches

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

On 07/15/2010 12:28 AM, Jeff Law wrote:
> On 07/14/10 15:30, Bernd Schmidt wrote:
>> If necessary I can try to test for a single insn with single_set and
>> push it to the sequence otherwise.
>>    
> For the mn103, the conversions were single insns...
> 
> Ultimately,  I think you have to peek at the insn(s) and see what
> registers they set/clobber.

How's this?  Bootstrapped and tested on i686-linux.  ARM tests in progress.


Bernd

[-- Attachment #2: more-equiv-notes2.diff --]
[-- Type: text/plain, Size: 5435 bytes --]

	PR target/42235
	* function.c (record_hard_reg_sets): New static function.
	(assign_parm_setup_reg): If an optab for extending exists and the
	generated code clobbbers no hard regs, emit the insn directly and
	create a REG_EQUIV note.

Index: gcc/function.c
===================================================================
--- gcc.orig/function.c
+++ gcc/function.c
@@ -2854,6 +2854,21 @@ assign_parm_setup_block (struct assign_p
   SET_DECL_RTL (parm, stack_parm);
 }
 
+/* A subroutine of assign_parm_setup_reg, called through note_stores.
+   This collects sets and clobbers of hard registers in a HARD_REG_SET,
+   which is pointed to by DATA.  */
+static void
+record_hard_reg_sets (rtx x, const_rtx pat ATTRIBUTE_UNUSED, void *data)
+{
+  HARD_REG_SET *pset = (HARD_REG_SET *)data;
+  if (REG_P (x) && REGNO (x) < FIRST_PSEUDO_REGISTER)
+    {
+      int nregs = hard_regno_nregs[REGNO (x)][GET_MODE (x)];
+      while (nregs-- > 0)
+	SET_HARD_REG_BIT (*pset, REGNO (x) + nregs);
+    }
+}
+
 /* A subroutine of assign_parms.  Allocate a pseudo to hold the current
    parameter.  Get it there.  Perform all ABI specified conversions.  */
 
@@ -2861,10 +2876,12 @@ static void
 assign_parm_setup_reg (struct assign_parm_data_all *all, tree parm,
 		       struct assign_parm_data_one *data)
 {
-  rtx parmreg;
+  rtx parmreg, validated_mem;
+  rtx equiv_stack_parm;
   enum machine_mode promoted_nominal_mode;
   int unsignedp = TYPE_UNSIGNED (TREE_TYPE (parm));
   bool did_conversion = false;
+  bool need_conversion, moved;
 
   /* Store the parm in a pseudoregister during the function, but we may
      need to do it in a wider mode.  Using 2 here makes the result
@@ -2893,11 +2910,16 @@ assign_parm_setup_reg (struct assign_par
 
   /* Copy the value into the register, thus bridging between
      assign_parm_find_data_types and expand_expr_real_1.  */
-  if (data->nominal_mode != data->passed_mode
-      || promoted_nominal_mode != data->promoted_mode)
-    {
-      int save_tree_used;
 
+  equiv_stack_parm = data->stack_parm;
+  validated_mem = validize_mem (data->entry_parm);
+
+  need_conversion = (data->nominal_mode != data->passed_mode
+		     || promoted_nominal_mode != data->promoted_mode);
+  moved = false;
+
+  if (need_conversion)
+    {
       /* ENTRY_PARM has been converted to PROMOTED_MODE, its
 	 mode, by the caller.  We now have to convert it to
 	 NOMINAL_MODE, if different.  However, PARMREG may be in
@@ -2913,13 +2935,70 @@ assign_parm_setup_reg (struct assign_par
 
 	 In addition, the conversion may involve a call, which could
 	 clobber parameters which haven't been copied to pseudo
-	 registers yet.  Therefore, we must first copy the parm to
-	 a pseudo reg here, and save the conversion until after all
+	 registers yet.
+
+	 First, we try to emit an insn which performs the necessary
+	 conversion.  We verify that this insn does not clobber any
+	 hard registers.  */
+
+      enum insn_code icode;
+      rtx op0, op1;
+
+      icode = can_extend_p (promoted_nominal_mode, data->passed_mode,
+			    unsignedp);
+
+      op0 = parmreg;
+      op1 = validated_mem;
+      if (icode != CODE_FOR_nothing
+	  && insn_data[icode].operand[0].predicate (op0, promoted_nominal_mode)
+	  && insn_data[icode].operand[1].predicate (op1, data->passed_mode))
+	{
+	  enum rtx_code code = unsignedp ? ZERO_EXTEND : SIGN_EXTEND;
+	  rtx insn, insns;
+	  HARD_REG_SET hardregs;
+
+	  start_sequence ();
+	  insn = gen_extend_insn (op0, op1, promoted_nominal_mode,
+				  data->passed_mode, unsignedp);
+	  emit_insn (insn);
+	  insns = get_insns ();
+
+	  moved = true;
+	  CLEAR_HARD_REG_SET (hardregs);
+	  for (insn = insns; insn && moved; insn = NEXT_INSN (insn))
+	    {
+	      if (INSN_P (insn))
+		note_stores (PATTERN (insn), record_hard_reg_sets,
+			     &hardregs);
+	      if (!hard_reg_set_empty_p (hardregs))
+		moved = false;
+	    }
+
+	  end_sequence ();
+
+	  if (moved)
+	    {
+	      emit_insn (insns);
+	      equiv_stack_parm = gen_rtx_fmt_e (code, GET_MODE (parmreg),
+						equiv_stack_parm);
+	    }
+	}
+    }
+
+  if (moved)
+    /* Nothing to do.  */
+    ;
+  else if (need_conversion)
+    {
+      /* We did not have an insn to convert directly, or the sequence
+	 generated appeared unsafe.  We must first copy the parm to a
+	 pseudo reg, and save the conversion until after all
 	 parameters have been moved.  */
 
+      int save_tree_used;
       rtx tempreg = gen_reg_rtx (GET_MODE (data->entry_parm));
 
-      emit_move_insn (tempreg, validize_mem (data->entry_parm));
+      emit_move_insn (tempreg, validated_mem);
 
       push_to_sequence2 (all->first_conversion_insn, all->last_conversion_insn);
       tempreg = convert_to_mode (data->nominal_mode, tempreg, unsignedp);
@@ -2949,7 +3028,7 @@ assign_parm_setup_reg (struct assign_par
       did_conversion = true;
     }
   else
-    emit_move_insn (parmreg, validize_mem (data->entry_parm));
+    emit_move_insn (parmreg, validated_mem);
 
   /* If we were passed a pointer but the actual value can safely live
      in a register, put it in one.  */
@@ -3034,7 +3113,7 @@ assign_parm_setup_reg (struct assign_par
 	}
       else if ((set = single_set (linsn)) != 0
 	       && SET_DEST (set) == parmreg)
-	set_unique_reg_note (linsn, REG_EQUIV, data->stack_parm);
+	set_unique_reg_note (linsn, REG_EQUIV, equiv_stack_parm);
     }
 
   /* For pointer data type, suggest pointer register.  */

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Emit more REG_EQUIV notes for function args (PR42235)
  2010-07-15 17:14       ` Bernd Schmidt
@ 2010-07-15 21:26         ` Jeff Law
  2010-07-19  9:55         ` Bernd Schmidt
  1 sibling, 0 replies; 15+ messages in thread
From: Jeff Law @ 2010-07-15 21:26 UTC (permalink / raw)
  To: Bernd Schmidt; +Cc: GCC Patches

On 07/15/10 11:13, Bernd Schmidt wrote:
> On 07/15/2010 12:28 AM, Jeff Law wrote:
>    
>> On 07/14/10 15:30, Bernd Schmidt wrote:
>>      
>>> If necessary I can try to test for a single insn with single_set and
>>> push it to the sequence otherwise.
>>>
>>>        
>> For the mn103, the conversions were single insns...
>>
>> Ultimately,  I think you have to peek at the insn(s) and see what
>> registers they set/clobber.
>>      
> How's this?  Bootstrapped and tested on i686-linux.  ARM tests in progress.
>    
OK.
Jeff

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Emit more REG_EQUIV notes for function args (PR42235)
  2010-07-15 17:14       ` Bernd Schmidt
  2010-07-15 21:26         ` Jeff Law
@ 2010-07-19  9:55         ` Bernd Schmidt
  2010-07-19 16:51           ` Jeff Law
  1 sibling, 1 reply; 15+ messages in thread
From: Bernd Schmidt @ 2010-07-19  9:55 UTC (permalink / raw)
  To: Jeff Law; +Cc: GCC Patches, John David Anglin

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

On 07/15/2010 07:13 PM, Bernd Schmidt wrote:
> On 07/15/2010 12:28 AM, Jeff Law wrote:
>> On 07/14/10 15:30, Bernd Schmidt wrote:
>>> If necessary I can try to test for a single insn with single_set and
>>> push it to the sequence otherwise.
>>>    
>> For the mn103, the conversions were single insns...
>>
>> Ultimately,  I think you have to peek at the insn(s) and see what
>> registers they set/clobber.
> 
> How's this?  Bootstrapped and tested on i686-linux.  ARM tests in progress.

This seems to have caused a bootstrap failure on hppa.  A QImode value
is passed in a SImode reg and then should get extended to DImode; we
must make sure we use a qidi extension.  The following patch does that;
I've not heard back whether it fixes the hppa bootstrap, but it seems to
work on arm-linux (one extra testsuite failure caused by an unrelated
change in the same tree).  Also bootstrapped on i686-linux.  Ok?


Bernd

[-- Attachment #2: entrymode.diff --]
[-- Type: text/plain, Size: 1376 bytes --]

	* function.c (assign_parm_setup_reg): Use nominal_mode as the
	source mode for extensions.

Index: function.c
===================================================================
--- function.c	(revision 162277)
+++ function.c	(working copy)
@@ -2940,18 +2940,18 @@ assign_parm_setup_reg (struct assign_par
 	 First, we try to emit an insn which performs the necessary
 	 conversion.  We verify that this insn does not clobber any
 	 hard registers.  */
-
+      enum machine_mode entry_mode = data->nominal_mode;
       enum insn_code icode;
       rtx op0, op1;
 
-      icode = can_extend_p (promoted_nominal_mode, data->passed_mode,
+      icode = can_extend_p (promoted_nominal_mode, entry_mode,
 			    unsignedp);
 
       op0 = parmreg;
       op1 = validated_mem;
       if (icode != CODE_FOR_nothing
 	  && insn_data[icode].operand[0].predicate (op0, promoted_nominal_mode)
-	  && insn_data[icode].operand[1].predicate (op1, data->passed_mode))
+	  && insn_data[icode].operand[1].predicate (op1, entry_mode))
 	{
 	  enum rtx_code code = unsignedp ? ZERO_EXTEND : SIGN_EXTEND;
 	  rtx insn, insns;
@@ -2959,7 +2959,7 @@ assign_parm_setup_reg (struct assign_par
 
 	  start_sequence ();
 	  insn = gen_extend_insn (op0, op1, promoted_nominal_mode,
-				  data->passed_mode, unsignedp);
+				  entry_mode, unsignedp);
 	  emit_insn (insn);
 	  insns = get_insns ();
 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Emit more REG_EQUIV notes for function args (PR42235)
  2010-07-19  9:55         ` Bernd Schmidt
@ 2010-07-19 16:51           ` Jeff Law
  2010-07-19 16:53             ` Bernd Schmidt
  2010-07-21 22:53             ` Bernd Schmidt
  0 siblings, 2 replies; 15+ messages in thread
From: Jeff Law @ 2010-07-19 16:51 UTC (permalink / raw)
  To: Bernd Schmidt; +Cc: GCC Patches, John David Anglin

On 07/19/10 03:54, Bernd Schmidt wrote:
> On 07/15/2010 07:13 PM, Bernd Schmidt wrote:
>    
>> On 07/15/2010 12:28 AM, Jeff Law wrote:
>>      
>>> On 07/14/10 15:30, Bernd Schmidt wrote:
>>>        
>>>> If necessary I can try to test for a single insn with single_set and
>>>> push it to the sequence otherwise.
>>>>
>>>>          
>>> For the mn103, the conversions were single insns...
>>>
>>> Ultimately,  I think you have to peek at the insn(s) and see what
>>> registers they set/clobber.
>>>        
>> How's this?  Bootstrapped and tested on i686-linux.  ARM tests in progress.
>>      
> This seems to have caused a bootstrap failure on hppa.  A QImode value
> is passed in a SImode reg and then should get extended to DImode; we
> must make sure we use a qidi extension.  The following patch does that;
> I've not heard back whether it fixes the hppa bootstrap, but it seems to
> work on arm-linux (one extra testsuite failure caused by an unrelated
> change in the same tree).  Also bootstrapped on i686-linux.  Ok?
>    
Assuming John confirms it fixes the PA failure.  OK.

jeff

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Emit more REG_EQUIV notes for function args (PR42235)
  2010-07-19 16:51           ` Jeff Law
@ 2010-07-19 16:53             ` Bernd Schmidt
  2010-07-19 16:59               ` Richard Henderson
                                 ` (2 more replies)
  2010-07-21 22:53             ` Bernd Schmidt
  1 sibling, 3 replies; 15+ messages in thread
From: Bernd Schmidt @ 2010-07-19 16:53 UTC (permalink / raw)
  To: Jeff Law; +Cc: GCC Patches, John David Anglin

On 07/19/2010 06:51 PM, Jeff Law wrote:
> Assuming John confirms it fixes the PA failure.  OK.

Yeah, it didn't, and I think I missed a gen_lowpart in there anyway :-(
 Now trying to do something with the compilefarm PA machine, although
I'm not sure it's a 64-bit one (is it?)


Bernd

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Emit more REG_EQUIV notes for function args (PR42235)
  2010-07-19 16:53             ` Bernd Schmidt
@ 2010-07-19 16:59               ` Richard Henderson
  2010-07-19 17:09               ` Jeff Law
  2010-07-19 19:06               ` John David Anglin
  2 siblings, 0 replies; 15+ messages in thread
From: Richard Henderson @ 2010-07-19 16:59 UTC (permalink / raw)
  To: Bernd Schmidt; +Cc: Jeff Law, GCC Patches, John David Anglin

On 07/19/2010 09:52 AM, Bernd Schmidt wrote:
> On 07/19/2010 06:51 PM, Jeff Law wrote:
>> Assuming John confirms it fixes the PA failure.  OK.
> 
> Yeah, it didn't, and I think I missed a gen_lowpart in there anyway :-(
>  Now trying to do something with the compilefarm PA machine, although
> I'm not sure it's a 64-bit one (is it?)

It's 64-bit hw running a 32-bit os, so... no, not really.


r~

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Emit more REG_EQUIV notes for function args (PR42235)
  2010-07-19 16:53             ` Bernd Schmidt
  2010-07-19 16:59               ` Richard Henderson
@ 2010-07-19 17:09               ` Jeff Law
  2010-07-19 19:06               ` John David Anglin
  2 siblings, 0 replies; 15+ messages in thread
From: Jeff Law @ 2010-07-19 17:09 UTC (permalink / raw)
  To: Bernd Schmidt; +Cc: GCC Patches, John David Anglin

On 07/19/10 10:52, Bernd Schmidt wrote:
> On 07/19/2010 06:51 PM, Jeff Law wrote:
>    
>> Assuming John confirms it fixes the PA failure.  OK.
>>      
> Yeah, it didn't, and I think I missed a gen_lowpart in there anyway :-(
>   Now trying to do something with the compilefarm PA machine, although
> I'm not sure it's a 64-bit one (is it?)
>    
No clue.  What does config.guess report?

If it reports hppa2.0w, then it's running in 64bit mode.  hppa2.0 is the 
same hardware running in 32bit mode.

My hppa2.0w machine is extremely flakey; last I tried, I couldn't get it 
to run long enough to get through stage1, so I can't be much direct help.

Jeff

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Emit more REG_EQUIV notes for function args (PR42235)
  2010-07-19 16:53             ` Bernd Schmidt
  2010-07-19 16:59               ` Richard Henderson
  2010-07-19 17:09               ` Jeff Law
@ 2010-07-19 19:06               ` John David Anglin
  2 siblings, 0 replies; 15+ messages in thread
From: John David Anglin @ 2010-07-19 19:06 UTC (permalink / raw)
  To: Bernd Schmidt; +Cc: law, gcc-patches

> On 07/19/2010 06:51 PM, Jeff Law wrote:
> > Assuming John confirms it fixes the PA failure.  OK.
> 
> Yeah, it didn't, and I think I missed a gen_lowpart in there anyway :-(
>  Now trying to do something with the compilefarm PA machine, although
> I'm not sure it's a 64-bit one (is it?)

I never use it.  HP-UX on "current" machines supports both 32 and 64-bit.
However, generating the initial 64-bit gcc version is still a bit tricky
if it is not present.  Linux only has a 32-bit runtime.

The failure was a null pointer fault.  However, it has been passed through
multiple layers of inlined functions, and gdb isn't working well.  So,
I haven't been able to find the fault.  I think I'll retry the testsuite
to see if I can find some more simple failures.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Emit more REG_EQUIV notes for function args (PR42235)
  2010-07-19 16:51           ` Jeff Law
  2010-07-19 16:53             ` Bernd Schmidt
@ 2010-07-21 22:53             ` Bernd Schmidt
  1 sibling, 0 replies; 15+ messages in thread
From: Bernd Schmidt @ 2010-07-21 22:53 UTC (permalink / raw)
  To: Jeff Law; +Cc: GCC Patches, John David Anglin

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

Here's a different patch, which I've committed as obvious after
bootstrapping and testing i686 and x64_64-linux.  It just restricts the
previous change to situations where it's clearly safe.


Bernd

[-- Attachment #2: parm-equiv-fix.diff --]
[-- Type: text/plain, Size: 1526 bytes --]

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 162390)
+++ ChangeLog	(working copy)
@@ -18,6 +18,10 @@
 	an old insn, ignore a use that occurs after store_ruid.
 	* Makefile.in (postreload.o): Update dependencies.
 
+	* function.c (record_hard_reg_sets): Restrict the previous change
+	to cases where the incoming nominal mode is the same as the
+	incoming promoted mode and everything happens in MODE_INT.
+
 2010-07-21  Jakub Jelinek  <jakub@redhat.com>
 
 	PR debug/45015
Index: function.c
===================================================================
--- function.c	(revision 162372)
+++ function.c	(working copy)
@@ -2918,7 +2918,10 @@ assign_parm_setup_reg (struct assign_par
 		     || promoted_nominal_mode != data->promoted_mode);
   moved = false;
 
-  if (need_conversion)
+  if (need_conversion
+      && GET_MODE_CLASS (data->nominal_mode) == MODE_INT
+      && data->nominal_mode == data->passed_mode
+      && data->nominal_mode == GET_MODE (data->entry_parm))
     {
       /* ENTRY_PARM has been converted to PROMOTED_MODE, its
 	 mode, by the caller.  We now have to convert it to
@@ -2979,8 +2982,9 @@ assign_parm_setup_reg (struct assign_par
 	  if (moved)
 	    {
 	      emit_insn (insns);
-	      equiv_stack_parm = gen_rtx_fmt_e (code, GET_MODE (parmreg),
-						equiv_stack_parm);
+	      if (equiv_stack_parm != NULL_RTX)
+		equiv_stack_parm = gen_rtx_fmt_e (code, GET_MODE (parmreg),
+						  equiv_stack_parm);
 	    }
 	}
     }

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2010-07-21 22:53 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-07-14 11:14 Emit more REG_EQUIV notes for function args (PR42235) Bernd Schmidt
2010-07-14 18:54 ` Jeff Law
2010-07-14 21:30   ` Bernd Schmidt
2010-07-14 22:28     ` Jeff Law
2010-07-15 17:14       ` Bernd Schmidt
2010-07-15 21:26         ` Jeff Law
2010-07-19  9:55         ` Bernd Schmidt
2010-07-19 16:51           ` Jeff Law
2010-07-19 16:53             ` Bernd Schmidt
2010-07-19 16:59               ` Richard Henderson
2010-07-19 17:09               ` Jeff Law
2010-07-19 19:06               ` John David Anglin
2010-07-21 22:53             ` Bernd Schmidt
2010-07-14 21:48   ` Bernd Schmidt
2010-07-14 22:22     ` Jeff Law

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).