#2087: Register allocator and lexicals bug
----------------------+-----------------------------------------------------
 Reporter:  jonathan  |       Owner:       
     Type:  bug       |      Status:  new  
 Priority:  blocker   |   Milestone:       
Component:  imcc      |     Version:  3.2.0
 Severity:  high      |    Keywords:       
     Lang:            |       Patch:  new  
 Platform:            |  
----------------------+-----------------------------------------------------
 Code:

 {{{
 }}}

 Should be a null PMC access, but prints "oh no". It looks like

 {{{
 .lex 'foo', $P0
 }}}

 isn't considered an instruction, but just saying "this lexical goes in
 this register". If that's the only mention of $P0, then there's no "first
 instruction". It builds a du-chain, then based on that throws away $P0
 from the "needs allocation" list because it confuses it with an unused
 register (e.g. it thinks it got optimized out, which is not the case at
 all here).

 This patch makes the issue go away:

 {{{
 diff --git a/compilers/imcc/reg_alloc.c b/compilers/imcc/reg_alloc.c
 index de0f3e0..300216d 100644
 --- a/compilers/imcc/reg_alloc.c
 +++ b/compilers/imcc/reg_alloc.c
 @@ -391,6 +391,9 @@ reg_sort_f(ARGIN(const void *a), ARGIN(const void *b))
      const SymReg * const ra = *(const SymReg * const *)a;
      const SymReg * const rb = *(const SymReg * const *)b;

 +    if (!ra->first_ins || !rb->first_ins)
 +        return 0;
 +
      if (ra->first_ins->index < rb->first_ins->index)
          return -1;

 @@ -473,7 +476,7 @@ build_reglist(PARROT_INTERP, ARGMOD(IMC_Unit *unit))

      /* we might have unused symbols here, from optimizations */
      for (i = count = unused = 0; i < n_symbols; i++) {
 -        if (!unit->reglist[i]->first_ins)
 +        if (!unit->reglist[i]->first_ins && !(unit->reglist[i]->usage &
 U_LEXICAL))
              unused++;
          else if (i == count)
              count++;
 @@ -716,7 +719,7 @@ vanilla_reg_alloc(PARROT_INTERP, ARGMOD(IMC_Unit
 *unit))
          SymReg *r;
          for (r = hsh->data[i]; r; r = r->next) {
              /* TODO Ignore non-volatiles */
 -            if (REG_NEEDS_ALLOC(r) && r->use_count)
 +            if (REG_NEEDS_ALLOC(r))
                  r->color = -1;
          }
      }
 @@ -732,7 +735,7 @@ vanilla_reg_alloc(PARROT_INTERP, ARGMOD(IMC_Unit
 *unit))
              for (r = hsh->data[i]; r; r = r->next) {
                  if (r->set != reg_set)
                      continue;
 -                if (REG_NEEDS_ALLOC(r) && (r->color == -1) &&
 r->use_count) {
 +                if (REG_NEEDS_ALLOC(r) && (r->color == -1)) {
                      if (set_contains(avail, first_reg))
                          first_reg = first_avail(interp, unit, reg_set,
 NULL);

 }}}

 But it's kinda hacky. OTOH, so is the register allocator as a whole, and
 this is blocking me on NQP, so provided we can be sure we don't make the
 quicksort of symbols to allocate unstable I'd be OK with something like
 this going in. Will leave it until tomorrow to see if anyone has any
 better ideas.

-- 
Ticket URL: <https://trac.parrot.org/parrot/ticket/2087>
Parrot <https://trac.parrot.org/parrot/>
Parrot Development
_______________________________________________
parrot-tickets mailing list
[email protected]
http://lists.parrot.org/mailman/listinfo/parrot-tickets

Reply via email to