diff mbox

Fix find_base_term in 32-bit -fpic code (PR lto/64025, take 2)

Message ID 20141126185413.GQ1669@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Nov. 26, 2014, 6:54 p.m. UTC 
On Tue, Nov 25, 2014 at 09:20:13AM +0100, Jakub Jelinek wrote:
> Actually, thinking about it more, at least according to
> commutative_operand_precedence the canonical order is
> what we used to return (i.e. (something - _G_O_T_) + (symbol_ref)
> or
> (something - _G_O_T_) + (const (symbol_ref +- const))
> So perhaps better fix is to follow find_base_value, which does something
> like:
>         /* Guess which operand is the base address:
>            If either operand is a symbol, then it is the base.  If
>            either operand is a CONST_INT, then the other is the base.  */
>         if (CONST_INT_P (src_1) || CONSTANT_P (src_0))
>           return find_base_value (src_0);
>         else if (CONST_INT_P (src_0) || CONSTANT_P (src_1))
>           return find_base_value (src_1);
> and do something similar in find_base_term too.  I.e. perhaps even with
> higher precedence over REG_P with REG_POINTER (or lower, in these cases
> it doesn't really matter, neither argument is REG_P), choose first
> operand that is CONSTANT_P and not CONST_INT_P.

Here it is.  Bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

2014-11-26  Jakub Jelinek  <jakub@redhat.com>

	PR lto/64025
	* alias.c (find_base_term): Use std::swap.  Prefer tmp2
	if it is CONSTANT_P other than CONST_INT.



	Jakub

Comments

Richard Biener Nov. 27, 2014, 10:04 a.m. UTC | #1 
On Wed, 26 Nov 2014, Jakub Jelinek wrote:

> On Tue, Nov 25, 2014 at 09:20:13AM +0100, Jakub Jelinek wrote:
> > Actually, thinking about it more, at least according to
> > commutative_operand_precedence the canonical order is
> > what we used to return (i.e. (something - _G_O_T_) + (symbol_ref)
> > or
> > (something - _G_O_T_) + (const (symbol_ref +- const))
> > So perhaps better fix is to follow find_base_value, which does something
> > like:
> >         /* Guess which operand is the base address:
> >            If either operand is a symbol, then it is the base.  If
> >            either operand is a CONST_INT, then the other is the base.  */
> >         if (CONST_INT_P (src_1) || CONSTANT_P (src_0))
> >           return find_base_value (src_0);
> >         else if (CONST_INT_P (src_0) || CONSTANT_P (src_1))
> >           return find_base_value (src_1);
> > and do something similar in find_base_term too.  I.e. perhaps even with
> > higher precedence over REG_P with REG_POINTER (or lower, in these cases
> > it doesn't really matter, neither argument is REG_P), choose first
> > operand that is CONSTANT_P and not CONST_INT_P.

Yeah, that makes sense.

> Here it is.  Bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?

Ok.

Thanks,
Richard.

> 2014-11-26  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR lto/64025
> 	* alias.c (find_base_term): Use std::swap.  Prefer tmp2
> 	if it is CONSTANT_P other than CONST_INT.
> 
> --- gcc/alias.c.jj	2014-11-21 10:17:17.000000000 +0100
> +++ gcc/alias.c	2014-11-26 12:31:24.719485590 +0100
> @@ -1756,11 +1756,11 @@ find_base_term (rtx x)
>  	if (REG_P (tmp1) && REG_POINTER (tmp1))
>  	  ;
>  	else if (REG_P (tmp2) && REG_POINTER (tmp2))
> -	  {
> -	    rtx tem = tmp1;
> -	    tmp1 = tmp2;
> -	    tmp2 = tem;
> -	  }
> +	  std::swap (tmp1, tmp2);
> +	/* If second argument is constant which has base term, prefer it
> +	   over variable tmp1.  See PR64025.  */
> +	else if (CONSTANT_P (tmp2) && !CONST_INT_P (tmp2))
> +	  std::swap (tmp1, tmp2);
>  
>  	/* Go ahead and find the base term for both operands.  If either base
>  	   term is from a pointer or is a named object or a special address
> 
> 
> 	Jakub
> 
>
diff mbox

Patch

--- gcc/alias.c.jj	2014-11-21 10:17:17.000000000 +0100
+++ gcc/alias.c	2014-11-26 12:31:24.719485590 +0100
@@ -1756,11 +1756,11 @@  find_base_term (rtx x)
 	if (REG_P (tmp1) && REG_POINTER (tmp1))
 	  ;
 	else if (REG_P (tmp2) && REG_POINTER (tmp2))
-	  {
-	    rtx tem = tmp1;
-	    tmp1 = tmp2;
-	    tmp2 = tem;
-	  }
+	  std::swap (tmp1, tmp2);
+	/* If second argument is constant which has base term, prefer it
+	   over variable tmp1.  See PR64025.  */
+	else if (CONSTANT_P (tmp2) && !CONST_INT_P (tmp2))
+	  std::swap (tmp1, tmp2);
 
 	/* Go ahead and find the base term for both operands.  If either base
 	   term is from a pointer or is a named object or a special address