Re: [PATCH] rs6000: Add basic support for prefixed and PC-relative addresses

[Segher Boessenkool <segher@kernel.crashing.org>]

On Wed, May 29, 2019 at 10:49:35PM -0500, Bill Schmidt wrote:
> 	* config/rs6000/predicates.md (pcrel_address): New
> 	define_predicate.

Please put that on one line?

> +      if (LABEL_REF_P (x))
> +	output_asm_label (x);
> +      else
> +	output_addr_const (file, x);

Why does LABEL_REF need separate handling here?

> +      if (offset)
> +	fprintf (file, "%s" HOST_WIDE_INT_PRINT_DEC, (offset > 0) ? "+" : "",
> +		 offset);

Maybe

      if (offset)
	fprintf (file, "%+" PRId64, offset);

(but there really should be a HOST_WIDE_INT_PRINT_DEC_PLUS or similar for
that then.  Oh well).

> +rs6000_prefixed_address (rtx addr, machine_mode mode)

> +  if (GET_CODE (addr) == PLUS)
> +    {
> +      HOST_WIDE_INT value, mask;

Please declare these where they are defined?

> +      /* DS instruction (bottom 2 bits must be 0).  For 32-bit integers, we
> +	 need to use DS instructions if we are sign-extending the value with
> +	 LWA.  For 32-bit floating point, we need DS instructions to load and
> +	 store values to the traditional Altivec registers.  */
> +      else if (GET_MODE_SIZE (mode) >= 4)
> +	mask = 3;

I don't much like penalising scalar single precision float like this.
But, this also hurts unaligned lwz...  Do we have statistics on that?
Offline, I guess :-)

> +      /* Return true if we must use a prefixed instruction.  */
> +      return ((value & ~mask) != value);

(value & mask) != 0


Okay with those things frobbed a little, or looked at.  Thanks!


Segher


Segher