Message ID | CABu31nMQ84-m74TBYvLa9bk8ARaBkoYyK3N5eprPUfe-4M4ssw@mail.gmail.com |
---|---|
State | New |
Headers | show |
On Wed, Dec 5, 2012 at 8:11 PM, Steven Bosscher wrote: > The function that triggers this, has this RTL after IRA: At which point, of course, I wanted to show after substituting hard regs for the pseudos, so that: > > 1: NOTE_INSN_DELETED > 3: NOTE_INSN_BASIC_BLOCK 2 > 2: NOTE_INSN_FUNCTION_BEG > 5: {r60:DI=frame:DI-0x10;clobber flags:CC;} > REG_UNUSED flags:CC > REG_EQUIV frame:DI-0x10 > 6: dx:DI=r60:DI > REG_DEAD r60:DI > REG_EQUAL frame:DI-0x10 > 7: si:SI=0x5 > 8: di:DI=`*.LC0' > 9: ax:QI=0 > 10: ax:SI=call [`printf'] argc:0 > REG_DEAD di:DI > REG_DEAD si:SI > REG_DEAD dx:DI > REG_UNUSED ax:SI > 15: ax:SI=0 > 18: use ax:SI becomes... 3: NOTE_INSN_BASIC_BLOCK 2 2: NOTE_INSN_FUNCTION_BEG 5: NOTE_INSN_DELETED 6: dx:DI=frame:DI REG_DEAD r60:DI REG_EQUAL frame:DI-0x10 7: si:SI=0x5 8: di:DI=`*.LC0' 9: ax:QI=0 10: ax:SI=call [`printf'] argc:0 REG_DEAD di:DI REG_DEAD si:SI REG_DEAD dx:DI REG_UNUSED ax:SI 15: ax:SI=0 18: use ax:SI on entry to lra_create_live_ranges(). > Note how there are no pseudos at all in the function. (... apart from the REG_DEAD use of r60. Both IRA and LRA sometimes leave REG_NOTEs referencing pseudos. I haven't figured out yet why.) Ciao! Steven
On 12-12-05 2:11 PM, Steven Bosscher wrote: > On Wed, Dec 5, 2012 at 7:47 PM, Vladimir Makarov wrote: >> On 12-12-05 1:13 PM, Aldy Hernandez wrote: >>> This is a division by zero ICE. >>> >>> In the testcase in the PR, both `n' and `lra_live_max_point' are zero. I >>> have opted to inhibit the dump when lra_live_max_point is zero, but I can >>> just as easily avoiding printing the percentage in this case. >>> >> Interesting. I never thought that is possible. >> >>> Let me know what you prefer. >>> >>> OK for trunk? >> Yes, it is ok if you add the testcase for GCC testsuite. > Eh, are you sure? > > This means we enter remove_some_program_points_and_update_live_ranges > with lra_live_max_point==0 (it's updated _after_ compression so at the > point of the dump it's using the value on entry, _before_ > compression). So we're doing things like: > > born = sbitmap_alloc (lra_live_max_point); // => sbitmap_alloc (0); > dead = sbitmap_alloc (lra_live_max_point); // idem > bitmap_clear (born); // clear a bitmap that doesn't really exit > bitmap_clear (dead); // idem. > max_regno = max_reg_num (); > for (i = FIRST_PSEUDO_REGISTER; i < (unsigned) max_regno; i++) > { // walk all registers on looking for live ranges -- but there > can't be any > for (r = lra_reg_info[i].live_ranges; r != NULL; r = r->next) > { > lra_assert (r->start <= r->finish); > bitmap_set_bit (born, r->start); > bitmap_set_bit (dead, r->finish); > } > } > > etc. > > > That makes no sense... > > The function that triggers this, has this RTL after IRA: > > 1: NOTE_INSN_DELETED > 3: NOTE_INSN_BASIC_BLOCK 2 > 2: NOTE_INSN_FUNCTION_BEG > 5: {r60:DI=frame:DI-0x10;clobber flags:CC;} > REG_UNUSED flags:CC > REG_EQUIV frame:DI-0x10 > 6: dx:DI=r60:DI > REG_DEAD r60:DI > REG_EQUAL frame:DI-0x10 > 7: si:SI=0x5 > 8: di:DI=`*.LC0' > 9: ax:QI=0 > 10: ax:SI=call [`printf'] argc:0 > REG_DEAD di:DI > REG_DEAD si:SI > REG_DEAD dx:DI > REG_UNUSED ax:SI > 15: ax:SI=0 > 18: use ax:SI > > Note how there are no pseudos at all in the function. So why run LRA > on it at all? The insn constraints could be not satisfied even if we have only hard registers. So we need to run LRA in any case. > How about the fix proposed below? It could work too. Either code is ok to me. The testscase is very rare and can be achieved only for a small program. So the code speed is not important. I favor a bit your code as it does not change the code presented in IRA and LRA which might be unified someday. So if you are ready to commit your patch before Aldy did it, please do it, Steven. > Ciao! > Steven > > > Index: lra-lives.c > =================================================================== > --- lra-lives.c (revision 194226) > +++ lra-lives.c (working copy) > @@ -915,6 +915,7 @@ lra_create_live_ranges (bool all_p) > basic_block bb; > int i, hard_regno, max_regno = max_reg_num (); > int curr_point; > + bool have_referenced_pseudos = false; > > timevar_push (TV_LRA_CREATE_LIVE_RANGES); > > @@ -947,10 +948,24 @@ lra_create_live_ranges (bool all_p) > lra_reg_info[i].call_p = false; > #endif > if (i >= FIRST_PSEUDO_REGISTER > - && lra_reg_info[i].nrefs != 0 && (hard_regno = reg_renumber[i]) >= 0) > - lra_hard_reg_usage[hard_regno] += lra_reg_info[i].freq; > + && lra_reg_info[i].nrefs != 0) > + { > + if ((hard_regno = reg_renumber[i]) >= 0) > + lra_hard_reg_usage[hard_regno] += lra_reg_info[i].freq; > + have_referenced_pseudos = true; > + } > } > lra_free_copies (); > + > + /* Under some circumstances, we can have functions without pseudo > + registers. For such functions, lra_live_max_point will be 0, > + see e.g. PR55604, and there's nothing more to do for us here. */ > + if (! have_referenced_pseudos) > + { > + timevar_pop (TV_LRA_CREATE_LIVE_RANGES); > + return; > + } > + > pseudos_live = sparseset_alloc (max_regno); > pseudos_live_through_calls = sparseset_alloc (max_regno); > pseudos_live_through_setjumps = sparseset_alloc (max_regno); > @@ -973,6 +988,7 @@ lra_create_live_ranges (bool all_p) > } > free (post_order_rev_cfg); > lra_live_max_point = curr_point; > + gcc_checking_assert (lra_live_max_point > 0); > if (lra_dump_file != NULL) > print_live_ranges (lra_dump_file); > /* Clean up. */
> It could work too. Either code is ok to me. The testscase is very rare > and can be achieved only for a small program. So the code speed is not > important. I favor a bit your code as it does not change the code > presented in IRA and LRA which might be unified someday. So if you are > ready to commit your patch before Aldy did it, please do it, Steven. Steven, go right ahead. Let me know when you commit so I can close the PR. Thanks.
On Wed, Dec 5, 2012 at 9:47 PM, Aldy Hernandez wrote:
> Steven, go right ahead. Let me know when you commit so I can close the PR.
Thanks, committed as r194230 (patch) and r194231 (test case) after
bootstrap&test on x86_64-unknown-linux-gnu.
Ciao!
Steven
Index: lra-lives.c =================================================================== --- lra-lives.c (revision 194226) +++ lra-lives.c (working copy) @@ -915,6 +915,7 @@ lra_create_live_ranges (bool all_p) basic_block bb; int i, hard_regno, max_regno = max_reg_num (); int curr_point; + bool have_referenced_pseudos = false; timevar_push (TV_LRA_CREATE_LIVE_RANGES); @@ -947,10 +948,24 @@ lra_create_live_ranges (bool all_p) lra_reg_info[i].call_p = false; #endif if (i >= FIRST_PSEUDO_REGISTER - && lra_reg_info[i].nrefs != 0 && (hard_regno = reg_renumber[i]) >= 0) - lra_hard_reg_usage[hard_regno] += lra_reg_info[i].freq; + && lra_reg_info[i].nrefs != 0) + { + if ((hard_regno = reg_renumber[i]) >= 0) + lra_hard_reg_usage[hard_regno] += lra_reg_info[i].freq; + have_referenced_pseudos = true; + } } lra_free_copies (); + + /* Under some circumstances, we can have functions without pseudo + registers. For such functions, lra_live_max_point will be 0, + see e.g. PR55604, and there's nothing more to do for us here. */ + if (! have_referenced_pseudos) + { + timevar_pop (TV_LRA_CREATE_LIVE_RANGES); + return; + } + pseudos_live = sparseset_alloc (max_regno); pseudos_live_through_calls = sparseset_alloc (max_regno); pseudos_live_through_setjumps = sparseset_alloc (max_regno); @@ -973,6 +988,7 @@ lra_create_live_ranges (bool all_p) } free (post_order_rev_cfg); lra_live_max_point = curr_point; + gcc_checking_assert (lra_live_max_point > 0); if (lra_dump_file != NULL) print_live_ranges (lra_dump_file); /* Clean up. */