| Message ID | 5076E75B.2000206@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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.
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
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. > > >
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); }
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.