Re: mips tls with -mlong64/-mgp32

[Richard Sandiford <richard at codesourcery dot com>]

DJ Delorie <dj@redhat.com> writes:
>> > check whether the offset is within the alignment of the SYMBOL_REF_DECL.
>
> Like this?
>
> Index: mips.c
> ===================================================================
> --- mips.c	(revision 124810)
> +++ mips.c	(working copy)
> @@ -1413,20 +1413,39 @@ mips_symbolic_constant_p (rtx x, enum mi
>  	 address, and we will apply a 16-bit offset after loading it.
>  	 If the symbol is local, the linker should provide enough local
>  	 GOT entries for a 16-bit offset, but larger offsets may lead
>  	 to GOT overflow.  */
>        return SMALL_INT (offset);
>  
> +    case SYMBOL_TPREL:
> +    case SYMBOL_DTPREL:
> +      {
> +	/* If for some reason we can't get the alignment for the
> +	   symbol, initializing this to zero means we won't accept any
> +	   offset.  */
> +	HOST_WIDE_INT align = 0;
> +	tree t;
> +
> +	/* Get the alignment of the symbol we're referring to.  */
> +	t = SYMBOL_REF_DECL (XVECEXP (x, 0, 0));

Although I agree that XVECEXP is right, it's a bit subtle.
Can you just change:

  if (UNSPEC_ADDRESS_P (x))
    *symbol_type = UNSPEC_ADDRESS_TYPE (x);

to:

  if (UNSPEC_ADDRESS_P (x))
    {
      *symbol_type = UNSPEC_ADDRESS_TYPE (x);
      x = UNSPEC_ADDRESS (x);
    }

instead, and use "x" here?  That'll make the whole case statement
more robust.

> +	if (t)
> +	  align = DECL_ALIGN_UNIT (t);
> +
> +	/* We accept offsets within the symbol's alignment, but not
> +	   beyond, because the linker can't handle TPREL hi/lo relocs
> +	   that involve carries.  */
> +	if (GET_CODE (offset) == CONST_INT
> +	    && INTVAL (offset) < align)
> +	  return true;
> +      }

Looks good otherwise.  Sorry to nitpick, but can you split it out into a
separate function for clarity?  Something like:

/* Return true if OFFSET is in the range [0, ALIGN), where ALIGN
   is the alignment of SYMBOL_REF X.  */

static bool
mips_offset_within_alignment_p (rtx x, HOST_WIDE_INT offset)

Can you also add a comment above the caller, like so:

/* There is no carry between the HI and LO REL relocations, so the
   offset is only valid if we know it won't lead to such a carry.  */

Also, you don't need to check whether GET_CODE (offset) == CONST_INT;
it always is.  (For avoidance of doubt, I don't think there's a strong
case for it being good defensively.  The function that provides OFFSET
is specifically defined to return a CONST_INT, and INTVAL will check
that anyway when RTL checking is enabled.)

Richard