Hi Vlad,

Vladimir Makarov <vmaka...@redhat.com> writes:
> @@ -543,23 +544,34 @@ extract_loc_address_regs (bool top_p, en
>           code1 = GET_CODE (arg1);
>         }
>  
> +     if (CONSTANT_P (arg0)
> +         || code1 == PLUS || code1 == MULT || code1 == ASHIFT)
> +       {
> +         tloc = arg1_loc;
> +         arg1_loc = arg0_loc;
> +         arg0_loc = tloc;
> +         arg0 = *arg0_loc;
> +         code0 = GET_CODE (arg0);
> +         arg1 = *arg1_loc;
> +         code1 = GET_CODE (arg1);
> +       }

When does this happen?  Is it from the extract_address_regs calls
in equiv_address_substitution, or somewhere else?

>       /* If this machine only allows one register per address, it
>          must be in the first operand.  */
>       if (MAX_REGS_PER_ADDRESS == 1 || code == LO_SUM)
>         {
> -         extract_loc_address_regs (false, mode, as, arg0_loc, false, code,
> -                                   code1, modify_p, ad);
>           lra_assert (ad->disp_loc == NULL);
>           ad->disp_loc = arg1_loc;
> +         extract_loc_address_regs (false, mode, as, arg0_loc, false, code,
> +                                   code1, modify_p, ad);
>         }
>       /* Base + disp addressing  */
> -     else if (code != PLUS && code0 != MULT && code0 != ASHIFT
> +     else if (code0 != PLUS && code0 != MULT && code0 != ASHIFT
>                && CONSTANT_P (arg1))
>         {
> -         extract_loc_address_regs (false, mode, as, arg0_loc, false, PLUS,
> -                                   code1, modify_p, ad);
>           lra_assert (ad->disp_loc == NULL);
>           ad->disp_loc = arg1_loc;
> +         extract_loc_address_regs (false, mode, as, arg0_loc, false, PLUS,
> +                                   code1, modify_p, ad);
>         }
>       /* If index and base registers are the same on this machine,
>          just record registers in any non-constant operands.  We

Wasn't sure: is the order of the disp_loc assignment and
extract_loc_address_regs call significant here, or did you just swap
them for cosmetic reasons?  The original order seemed stronger on the
face of it, because we assert that the recursive call hasn't also
installed a displacement.

> @@ -624,11 +635,18 @@ extract_loc_address_regs (bool top_p, en
>  
>      case MULT:
>      case ASHIFT:
> -      extract_loc_address_regs (false, mode, as, &XEXP (*loc, 0), true,
> -                             outer_code, code, modify_p, ad);
> -      lra_assert (ad->index_loc == NULL);
> -      ad->index_loc = loc;
> -      break;
> +      {
> +     rtx *arg0_loc = &XEXP (x, 0);
> +     enum rtx_code code0 = GET_CODE (*arg0_loc);
> +     
> +     if (code0 == CONST_INT)
> +       arg0_loc = &XEXP (x, 1);
> +     extract_loc_address_regs (false, mode, as, arg0_loc, true,
> +                               outer_code, code, modify_p, ad);
> +     lra_assert (ad->index_loc == NULL);
> +     ad->index_loc = loc;
> +     break;
> +      }

Is this just for the MULT case?  Treating X in (ashift 1 X) as an
index register seems odd on the face of it.

> @@ -1757,10 +1786,10 @@ process_alt_operands (int only_alternati
>                          clobber operand if the matching operand is
>                          not dying in the insn.  */
>                       if (! curr_static_id->operand[m].early_clobber
> -                         /*|| operand_reg[nop] == NULL_RTX
> +                         || operand_reg[nop] == NULL_RTX
>                           || (find_regno_note (curr_insn, REG_DEAD,
>                                                REGNO (operand_reg[nop]))
> -                                              != NULL_RTX)*/)
> +                                              != NULL_RTX))
>                         match_p = true;
>                     }
>                   if (match_p)

Ah, so did you find the reason why this was done? :-)

> Index: lra.c
> ===================================================================
> --- lra.c     (revision 192634)
> +++ lra.c     (working copy)
> @@ -2035,7 +2035,8 @@ check_rtl (bool final_p)
>                  as legitimate.  Although they are legitimate if
>                  they satisfies the constraints and will be checked
>                  by insn constraints which we ignore here.  */
> -             && GET_RTX_CLASS (GET_CODE (XEXP (op, 0))) == RTX_AUTOINC)
> +             && GET_CODE (XEXP (op, 0)) != UNSPEC
> +             && GET_RTX_CLASS (GET_CODE (XEXP (op, 0))) != RTX_AUTOINC)
>             fatal_insn_not_found (insn);
>         }
>        }

Which case was this needed for?  RTX_AUTOINC codes really did seem like
a special case because they have their own constraints ("<" and ">").
But why do we have to assume that all unspec addresses are inherently valid?

Richard

Reply via email to