Re: 4.3 Patch for bad code generation on HPPA platform

["Richard Guenther" <richard dot guenther at gmail dot com>]

On Thu, May 8, 2008 at 10:14 PM, Steve Ellcey <sje@cup.hp.com> wrote:
> I would like approval to backport the following patch(es) to the 4.3
>  branch.  The original patch was done on the main line as a performance
>  improvement for Power and was not backported because it wasn't fixing a
>  regression.  But the patch does fix a regression on the hppa platform
>  that was introduced by an earlier patch and without this fix GCC 4.3.0
>  cannot correctly compile the SPEC 2000 GCC benchmark.  It will compile
>  it but then we get a core dump during execution because the optimizer
>  loses track of what is and isn't a pointer.  GCC does correctly compile
>  this benchmark with the GCC 4.2 branch and with the ToT sources.
>
>  After the initial patch was checked in part of it was backed out and
>  then replaced by a new piece of code.  This patch includes all three of
>  those changes combined into one patch.  They were checked in on the main
>  line at versions 133985, 134095, and 134139.
>
>  See http://gcc.gnu.org/ml/gcc/2008-05/msg00068.html for more information
>  on the HPPA problem.
>
>  I have verified this fixes the SPEC 2000 GCC benchmark problem on
>  hppa64-hp-hpux11.11 and causes no regressions there.  I also did a
>  x86_64-redhat-linux-gnu bootstrap and gcc test run and checked that
>  there were no regressions there either.
>
>  OK to backport it to the GCC 4.3 branch?

This is ok.

Thanks,
Richard.

>  Steve Ellcey
>  sje@cup.hp.com
>
>
>
>
>  2008-05-07  Steve Ellcey  <sje@cup.hp.com>
>
>         Backport from mainline:
>
>         2008-04-07  Peter Bergner  <bergner@vnet.ibm.com>
>
>         PR middle-end/PR28690
>         * rtlanal.c: Update copyright years.
>         (commutative_operand_precedence): Give SYMBOL_REF's the same precedence
>         as REG_POINTER and MEM_POINTER operands.
>         * emit-rtl.c (gen_reg_rtx_and_attrs): New function.
>         (set_reg_attrs_from_value): Call mark_reg_pointer as appropriate.
>         * rtl.h (gen_reg_rtx_and_attrs): Add prototype for new function.
>         * gcse.c: Update copyright years.
>         (pre_delete): Call gen_reg_rtx_and_attrs.
>         (hoist_code): Likewise.
>         (build_store_vectors): Likewise.
>         (delete_store): Likewise.
>         * loop-invariant.c (move_invariant_reg): Likewise.
>         Update copyright years.
>
>         2008-04-08  Peter Bergner  <bergner@vnet.ibm.com>
>
>         Revert
>         PR middle-end/PR28690
>         * rtlanal.c: (commutative_operand_precedence): Give SYMBOL_REF's the
>         same precedence as REG_POINTER and MEM_POINTER operands.
>
>         2008-04-09  Peter Bergner  <bergner@vnet.ibm.com>
>
>         PR middle-end/PR28690
>         * explow.c (break_out_memory_refs): Use simplify_gen_binary rather
>         than gen_rtx_fmt_ee to perform more canonicalizations.
>
>
>  Index: gcse.c
>  ===================================================================
>  --- gcse.c      (revision 135046)
>  +++ gcse.c      (working copy)
>  @@ -4456,8 +4456,7 @@ pre_delete (void)
>                    expressions into.  Get the mode for the new pseudo from
>                    the mode of the original destination pseudo.  */
>                 if (expr->reaching_reg == NULL)
>  -                 expr->reaching_reg
>  -                   = gen_reg_rtx (GET_MODE (SET_DEST (set)));
>  +                 expr->reaching_reg = gen_reg_rtx_and_attrs (SET_DEST (set));
>
>                 gcse_emit_move_after (expr->reaching_reg, SET_DEST (set), insn);
>                 delete_insn (insn);
>  @@ -4981,7 +4980,7 @@ hoist_code (void)
>                          from the mode of the original destination pseudo.  */
>                       if (expr->reaching_reg == NULL)
>                         expr->reaching_reg
>  -                         = gen_reg_rtx (GET_MODE (SET_DEST (set)));
>  +                         = gen_reg_rtx_and_attrs (SET_DEST (set));
>
>                       gcse_emit_move_after (expr->reaching_reg, SET_DEST (set), insn);
>                       delete_insn (insn);
>  @@ -6114,7 +6113,7 @@ build_store_vectors (void)
>              are any side effects.  */
>           if (TEST_BIT (ae_gen[bb->index], ptr->index))
>             {
>  -             rtx r = gen_reg_rtx (GET_MODE (ptr->pattern));
>  +             rtx r = gen_reg_rtx_and_attrs (ptr->pattern);
>               if (dump_file)
>                 fprintf (dump_file, "Removing redundant store:\n");
>               replace_store_insn (r, XEXP (st, 0), bb, ptr);
>  @@ -6437,7 +6436,7 @@ delete_store (struct ls_expr * expr, bas
>    rtx reg, i, del;
>
>    if (expr->reaching_reg == NULL_RTX)
>  -    expr->reaching_reg = gen_reg_rtx (GET_MODE (expr->pattern));
>  +    expr->reaching_reg = gen_reg_rtx_and_attrs (expr->pattern);
>
>    reg = expr->reaching_reg;
>
>  Index: emit-rtl.c
>  ===================================================================
>  --- emit-rtl.c  (revision 135046)
>  +++ emit-rtl.c  (working copy)
>  @@ -961,11 +961,32 @@ set_reg_attrs_from_value (rtx reg, rtx x
>    int offset;
>
>    offset = byte_lowpart_offset (GET_MODE (reg), GET_MODE (x));
>  -  if (MEM_P (x) && MEM_OFFSET (x) && GET_CODE (MEM_OFFSET (x)) == CONST_INT)
>  -    REG_ATTRS (reg)
>  -      = get_reg_attrs (MEM_EXPR (x), INTVAL (MEM_OFFSET (x)) + offset);
>  -  if (REG_P (x) && REG_ATTRS (x))
>  -    update_reg_offset (reg, x, offset);
>  +  if (MEM_P (x))
>  +    {
>  +      if (MEM_OFFSET (x) && GET_CODE (MEM_OFFSET (x)) == CONST_INT)
>  +       REG_ATTRS (reg)
>  +         = get_reg_attrs (MEM_EXPR (x), INTVAL (MEM_OFFSET (x)) + offset);
>  +      if (MEM_POINTER (x))
>  +       mark_reg_pointer (reg, MEM_ALIGN (x));
>  +    }
>  +  else if (REG_P (x))
>  +    {
>  +      if (REG_ATTRS (x))
>  +       update_reg_offset (reg, x, offset);
>  +      if (REG_POINTER (x))
>  +       mark_reg_pointer (reg, REGNO_POINTER_ALIGN (REGNO (x)));
>  +    }
>  +}
>  +
>  +/* Generate a REG rtx for a new pseudo register, copying the mode
>  +   and attributes from X.  */
>  +
>  +rtx
>  +gen_reg_rtx_and_attrs (rtx x)
>  +{
>  +  rtx reg = gen_reg_rtx (GET_MODE (x));
>  +  set_reg_attrs_from_value (reg, x);
>  +  return reg;
>   }
>
>   /* Set the register attributes for registers contained in PARM_RTX.
>  Index: explow.c
>  ===================================================================
>  --- explow.c    (revision 135046)
>  +++ explow.c    (working copy)
>  @@ -305,7 +305,7 @@ break_out_memory_refs (rtx x)
>        rtx op1 = break_out_memory_refs (XEXP (x, 1));
>
>        if (op0 != XEXP (x, 0) || op1 != XEXP (x, 1))
>  -       x = gen_rtx_fmt_ee (GET_CODE (x), Pmode, op0, op1);
>  +       x = simplify_gen_binary (GET_CODE (x), Pmode, op0, op1);
>      }
>
>    return x;
>  Index: loop-invariant.c
>  ===================================================================
>  --- loop-invariant.c    (revision 135046)
>  +++ loop-invariant.c    (working copy)
>  @@ -1193,7 +1193,7 @@ move_invariant_reg (struct loop *loop, u
>          need to create a temporary register.  */
>        set = single_set (inv->insn);
>        dest = SET_DEST (set);
>  -      reg = gen_reg_rtx (GET_MODE (dest));
>  +      reg = gen_reg_rtx_and_attrs (dest);
>
>        /* Try replacing the destination by a new pseudoregister.  */
>        if (!validate_change (inv->insn, &SET_DEST (set), reg, false))
>  Index: rtl.h
>  ===================================================================
>  --- rtl.h       (revision 135046)
>  +++ rtl.h       (working copy)
>  @@ -1509,6 +1509,7 @@ extern rtvec gen_rtvec_v (int, rtx *);
>   extern rtx gen_reg_rtx (enum machine_mode);
>   extern rtx gen_rtx_REG_offset (rtx, enum machine_mode, unsigned int, int);
>   extern rtx gen_reg_rtx_offset (rtx, enum machine_mode, int);
>  +extern rtx gen_reg_rtx_and_attrs (rtx);
>   extern rtx gen_label_rtx (void);
>   extern rtx gen_lowpart_common (enum machine_mode, rtx);
>
>