Re: [PATCH, rtl-optimization]: FIX PR 36111

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

On Mon, May 12, 2008 at 3:08 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> Hello!
>
>  From gcse.c, try_replace_reg (), we enter validate_src_group () with
>  following arguments:
>
>  insn:
>  (insn 8 20 0 3 pr36111.c:14 (parallel [
>             (set (mem/s/c:SI (plus:SI (reg/f:SI 60)
>                         (reg:SI 3 bx)) [3 main_arena.lock+0 S4 A32])
>                 (asm_operands/v:SI ("") ("=m") 0 [
>                         (mem/s/c:SI (plus:SI (reg/f:SI 60)
>                                 (reg:SI 3 bx)) [3 main_arena.lock+0 S4 A32])
>                     ]
>                      [
>                         (asm_input:SI ("m") 0)
>                     ] 1741))
>             (clobber (reg:QI 18 fpsr))
>             (clobber (reg:QI 17 flags))
>         ]) -1 (expr_list:REG_DEAD (reg/f:SI 58)
>         (expr_list:REG_UNUSED (reg:QI 18 fpsr)
>             (expr_list:REG_UNUSED (reg:QI 17 flags)
>                 (nil)))))
>
>  from:
>  (reg/f:SI 60)
>
>  to:
>  (const:SI (unspec:SI [
>             (symbol_ref:SI ("main_arena") [flags 0x2] <var_decl
>  0xb7cc9000 main_arena>)
>         ] 1))
>
>
>  Please note two (reg 60) pseudos inside the first element of (insn 8)
>  parallel that we are about to replace. note_uses () loops over all
>  elements of a parallel RTX and calls validate_replace_src_1 () that
>  passes arguments to validate_replace_rtx_1 (). We start with:
>
>             (set (mem/s/c:SI (plus:SI (reg/f:SI 60)
>                         (reg:SI 3 bx)) [3 main_arena.lock+0 S4 A32])
>                 (asm_operands/v:SI ("") ("=m") 0 [
>                         (mem/s/c:SI (plus:SI (reg/f:SI 60)
>                                 (reg:SI 3 bx)) [3 main_arena.lock+0 S4 A32])
>                     ]
>                      [
>                         (asm_input:SI ("m") 0)
>                     ] 1741))
>
>
>  Here is the problem:
>
>  When validate_replace_rtx_1 () loops over its arguments, it
>  substitutes both instances of (reg 60) pseudo. Following the
>  individual replacement, compilation enters
>
>   if (SWAPPABLE_OPERANDS_P (x)
>       && swap_commutative_operands_p (XEXP (x, 0), XEXP (x, 1)))
>     {
>       validate_unshare_change (object, loc,
>                                gen_rtx_fmt_ee (COMMUTATIVE_ARITH_P (x) ? code
>                                                : swap_condition (code),
>                                                GET_MODE (x), XEXP (x, 1),
>                                                XEXP (x, 0)), 1);
>
>  and here we generate new RTL expression that directly references XEXP
>  (x, 0) in order to create canonical RTX from from just created
>  expression with substituted operand:
>
>  (plus:SI (const:SI (unspec:SI [
>                 (symbol_ref:SI ("main_arena") [flags 0x2] <var_decl
>  0xb7d15000 main_arena>)
>             ] 1))
>     (reg:SI 3 bx))
>
>  This canonical RTX looks like:
>
>  (plus:SI (reg:SI 3 bx)
>     (const:SI (unspec:SI [
>                 (symbol_ref:SI ("main_arena") [flags 0x2] <var_decl
>  0xb7d15000 main_arena>)
>             ] 1)))
>
>
>  Since this happens for both pseudos that reference (reg 60), invalid
>  shared RTX is created.
>
>  The solution is to mark generated RTX for unsharing.
>
>  Attached patch implement this solution.
>
>  2008-05-09  Uros Bizjak  <ubizjak@gmail.com>
>
>         PR rtl-optimization/36111
>         * recog.c (validate_replace_rtx_1): Unshare new RTL expression
>         that was created for swappable operands.
>
>  testsuite/ChangeLog:
>
>  2008-05-09  Uros Bizjak  <ubizjak@gmail.com>
>
>         PR rtl-optimization/36111
>         * gcc.dg/pr36111.c: New test.
>
>  The patch was regression tested on i686-pc-linux-gnu and in addition
>  to fixing attached testcase, it also fixes all sse3-* -O2 -fpic
>  failures, found at [1].
>
>  OK for mainline?

Ok.

Thanks,
Richard.

>  [1] http://gcc.gnu.org/ml/gcc-testresults/2008-05/msg01014.html
>
>  Uros.
>