Patchwork sel-sched: fix merging of LHS reg availability (PR 50340)

login
register
mail settings
Submitter Alexander Monakov
Date Sept. 13, 2011, 4:42 p.m.
Message ID <alpine.LNX.2.00.1109132024060.28498@monoid.intra.ispras.ru>
Download mbox | patch
Permalink /patch/114511/
State New
Headers show

Comments

Alexander Monakov - Sept. 13, 2011, 4:42 p.m.
Hello,

This patches fixes an ICE on an assert that performs a sanity check on
target_available field of expr_t, which is tri-state: LHS register is
available (1), not available (0) or unknown (-1).

The problem is, when merging expr data of separable exprs with differing
LHSes, we can't claim we know anything about availability of the target
register if the unavailable LHS of the other expr is not the same register.
Thus, we should set the field to -1, not 0.

Fixed as follows, bootstrapped and regtested on x86_64-linux and ia64-linux
(without java, with one recent SRA patch reverted to unbreak bootstrap) with
sel-sched enabled at -O2.  OK for trunk?

(a small testcase is not available at the moment, but I can try to produce one
using delta before committing)


2011-09-13  Andrey Belevantsev  <abel@ispras.ru>

	* sel-sched-ir.c (update_target_availability): LHS register
	availability is not known if the unavailable LHS of the other
	expression is a different register.
Alexander Monakov - Oct. 10, 2011, 3:30 p.m.
Ping.  This is now a P1 bug, as it breaks spec2k gcc on ia64.

On Tue, 13 Sep 2011, Alexander Monakov wrote:

> Hello,
> 
> This patches fixes an ICE on an assert that performs a sanity check on
> target_available field of expr_t, which is tri-state: LHS register is
> available (1), not available (0) or unknown (-1).
> 
> The problem is, when merging expr data of separable exprs with differing
> LHSes, we can't claim we know anything about availability of the target
> register if the unavailable LHS of the other expr is not the same register.
> Thus, we should set the field to -1, not 0.
> 
> Fixed as follows, bootstrapped and regtested on x86_64-linux and ia64-linux
> (without java, with one recent SRA patch reverted to unbreak bootstrap) with
> sel-sched enabled at -O2.  OK for trunk?
> 
> (a small testcase is not available at the moment, but I can try to produce one
> using delta before committing)
> 
> 
> 2011-09-13  Andrey Belevantsev  <abel@ispras.ru>
> 
> 	* sel-sched-ir.c (update_target_availability): LHS register
> 	availability is not known if the unavailable LHS of the other
> 	expression is a different register.
> 
> diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
> index 4878460..b132392 100644
> --- a/gcc/sel-sched-ir.c
> +++ b/gcc/sel-sched-ir.c
> @@ -1746,7 +1746,13 @@ update_target_availability (expr_t to, expr_t from,
> insn_t split_point)
>              EXPR_TARGET_AVAILABLE (to) = -1;
>          }
>        else
> -        EXPR_TARGET_AVAILABLE (to) &= EXPR_TARGET_AVAILABLE (from);
> +    if (EXPR_TARGET_AVAILABLE (from) == 0
> +        && EXPR_LHS (from)
> +        && REG_P (EXPR_LHS (from))
> +        && REGNO (EXPR_LHS (to)) != REGNO (EXPR_LHS (from)))
> +      EXPR_TARGET_AVAILABLE (to) = -1;
> +    else
> +      EXPR_TARGET_AVAILABLE (to) &= EXPR_TARGET_AVAILABLE (from);
>      }
>  }
>
Vladimir Makarov - Oct. 14, 2011, 3:45 p.m.
On 09/13/2011 12:42 PM, Alexander Monakov wrote:
> Fixed as follows, bootstrapped and regtested on x86_64-linux and ia64-linux
> (without java, with one recent SRA patch reverted to unbreak bootstrap) with
> sel-sched enabled at -O2.  OK for trunk?
>
Ok with a small code format change below.
> (a small testcase is not available at the moment, but I can try to produce one
> using delta before committing)
>
>
> 2011-09-13  Andrey Belevantsev<abel@ispras.ru>
>
> 	* sel-sched-ir.c (update_target_availability): LHS register
> 	availability is not known if the unavailable LHS of the other
> 	expression is a different register.
>
> diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
> index 4878460..b132392 100644
> --- a/gcc/sel-sched-ir.c
> +++ b/gcc/sel-sched-ir.c
> @@ -1746,7 +1746,13 @@ update_target_availability (expr_t to, expr_t from,
> insn_t split_point)
>               EXPR_TARGET_AVAILABLE (to) = -1;
>           }
>         else
> -        EXPR_TARGET_AVAILABLE (to)&= EXPR_TARGET_AVAILABLE (from);
Please, put else and if on the same line with a proper indentation.
> +    if (EXPR_TARGET_AVAILABLE (from) == 0
> +&&  EXPR_LHS (from)
> +&&  REG_P (EXPR_LHS (from))
> +&&  REGNO (EXPR_LHS (to)) != REGNO (EXPR_LHS (from)))
> +      EXPR_TARGET_AVAILABLE (to) = -1;
> +    else
> +      EXPR_TARGET_AVAILABLE (to)&= EXPR_TARGET_AVAILABLE (from);
>       }
>   }

Patch

diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
index 4878460..b132392 100644
--- a/gcc/sel-sched-ir.c
+++ b/gcc/sel-sched-ir.c
@@ -1746,7 +1746,13 @@  update_target_availability (expr_t to, expr_t from,
insn_t split_point)
             EXPR_TARGET_AVAILABLE (to) = -1;
         }
       else
-        EXPR_TARGET_AVAILABLE (to) &= EXPR_TARGET_AVAILABLE (from);
+    if (EXPR_TARGET_AVAILABLE (from) == 0
+        && EXPR_LHS (from)
+        && REG_P (EXPR_LHS (from))
+        && REGNO (EXPR_LHS (to)) != REGNO (EXPR_LHS (from)))
+      EXPR_TARGET_AVAILABLE (to) = -1;
+    else
+      EXPR_TARGET_AVAILABLE (to) &= EXPR_TARGET_AVAILABLE (from);
     }
 }