Patchwork [lra] patch to fix GCC crash on a SPEC2006 test

login
register
mail settings
Submitter Vladimir Makarov
Date Oct. 11, 2012, 3:35 p.m.
Message ID <5076E75B.2000206@redhat.com>
Download mbox | patch
Permalink /patch/190931/
State New
Headers show

Comments

Vladimir Makarov - Oct. 11, 2012, 3:35 p.m.
The following patch fixes scheduler crash on a SPEC2006 test because LRA 
did not process CALL_INSN_FUNCTION_USAGE.

The patch was successfully bootstrapped and tested on x86/x86-64.

Committed as rev. 192362.

2012-10-11  Vladimir Makarov  <vmakarov@redhat.com>

         * lra-spills.c (spill_pseudos): Process CALL_INSN_FUNCTION_USAGE
         too.
Peter Bergner - Oct. 11, 2012, 4:56 p.m.
On Thu, 2012-10-11 at 11:35 -0400, Vladimir Makarov wrote:
> +         /* Call insn might have not references for pseudos besides
> +            in CALL_INSN_FUNCTION_USAGE but we don't count them in
> +            insn_bitmap of corresponding lra_reg_info as they don't
> +            need reloads.  */

Was this supposed to read:

  Call insn might have references of pseudos besides those in
  CALL_INSN_FUNCTION_USAGE, but we don't count them in insn_bitmap
  of the corresponding lra_reg_info as they don't need reloads.

???

Peter
Vladimir Makarov - Oct. 12, 2012, 3:53 a.m.
On 12-10-11 12:56 PM, Peter Bergner wrote:
> On Thu, 2012-10-11 at 11:35 -0400, Vladimir Makarov wrote:
>> +         /* Call insn might have not references for pseudos besides
>> +            in CALL_INSN_FUNCTION_USAGE but we don't count them in
>> +            insn_bitmap of corresponding lra_reg_info as they don't
>> +            need reloads.  */
> Was this supposed to read:
>
>    Call insn might have references of pseudos besides those in
>    CALL_INSN_FUNCTION_USAGE, but we don't count them in insn_bitmap
>    of the corresponding lra_reg_info as they don't need reloads.
>
My biggest problem is ESL.  I should use simpler phrases.

Is the following comment better?

Presence of any pseudo in CALL_INSN_FUNCTION_USAGE does not affect value 
of insn_bitmap of the corresponding lra_reg_info.  That is because we 
don't need to reload pseudos in CALL_INSN_FUNCTION_USAGEs.  So if we 
process only insns in the insn_bitmap of given pseudo here, we can miss 
the pseudo in some CALL_INSN_FUNCTION_USAGEs.
Peter Bergner - Oct. 13, 2012, 3:37 p.m.
On Thu, 2012-10-11 at 23:53 -0400, Vladimir Makarov wrote:
> My biggest problem is ESL.  I should use simpler phrases.

Heh, I'm not sure compiler speak qualifies as English. :)


> Is the following comment better?
> 
> Presence of any pseudo in CALL_INSN_FUNCTION_USAGE does not affect value 
> of insn_bitmap of the corresponding lra_reg_info.  That is because we 
> don't need to reload pseudos in CALL_INSN_FUNCTION_USAGEs.  So if we 
> process only insns in the insn_bitmap of given pseudo here, we can miss 
> the pseudo in some CALL_INSN_FUNCTION_USAGEs.

Sure, that's better.  Thanks.

Peter
Vladimir Makarov - Oct. 14, 2012, 5:49 p.m.
On 12-10-13 11:37 AM, Peter Bergner wrote:
> On Thu, 2012-10-11 at 23:53 -0400, Vladimir Makarov wrote:
>> Is the following comment better?
>>
>> Presence of any pseudo in CALL_INSN_FUNCTION_USAGE does not affect value
>> of insn_bitmap of the corresponding lra_reg_info.  That is because we
>> don't need to reload pseudos in CALL_INSN_FUNCTION_USAGEs.  So if we
>> process only insns in the insn_bitmap of given pseudo here, we can miss
>> the pseudo in some CALL_INSN_FUNCTION_USAGEs.
> Sure, that's better.  Thanks.
Ok.  Fixed.
>
>
>

Patch

Index: lra-spills.c
===================================================================
--- lra-spills.c	(revision 192341)
+++ lra-spills.c	(working copy)
@@ -480,6 +480,8 @@  spill_pseudos (void)
 	if (bitmap_bit_p (&changed_insns, INSN_UID (insn)))
 	  {
 	    remove_pseudos (&PATTERN (insn), insn);
+	    if (CALL_P (insn))
+	      remove_pseudos (&CALL_INSN_FUNCTION_USAGE (insn), insn);
 	    if (lra_dump_file != NULL)
 	      fprintf (lra_dump_file,
 		       "Changing spilled pseudos to memory in insn #%u\n",
@@ -488,6 +490,12 @@  spill_pseudos (void)
 	    if (lra_reg_spill_p || targetm.different_addr_displacement_p ())
 	      lra_set_used_insn_alternative (insn, -1);
 	  }
+	else if (CALL_P (insn))
+	  /* Call insn might have not references for pseudos besides
+	     in CALL_INSN_FUNCTION_USAGE but we don't count them in
+	     insn_bitmap of corresponding lra_reg_info as they don't
+	     need reloads.  */
+	  remove_pseudos (&CALL_INSN_FUNCTION_USAGE (insn), insn);
       bitmap_and_compl_into (DF_LR_IN (bb), &spilled_pseudos);
       bitmap_and_compl_into (DF_LR_OUT (bb), &spilled_pseudos);
     }