Re: [RFA] [V3] new pass for sign/zero extension elimination

[Richard Sandiford <richard.sandiford@arm.com>]

Richard Sandiford <richard.sandiford@arm.com> writes:
> Jeff Law <jlaw@ventanamicro.com> writes:
>> [...]
>> +	  if (GET_CODE (x) == ZERO_EXTRACT)
>> +	    {
>> +	      /* If either the size or the start position is unknown,
>> +		 then assume we know nothing about what is overwritten.
>> +		 This is overly conservative, but safe.  */
>> +	      if (!CONST_INT_P (XEXP (x, 1)) || !CONST_INT_P (XEXP (x, 2)))
>> +		continue;
>> +	      mask = (1ULL << INTVAL (XEXP (x, 1))) - 1;
>> +	      bit = INTVAL (XEXP (x, 2));
>> +	      if (BITS_BIG_ENDIAN)
>> +		bit = (GET_MODE_BITSIZE (GET_MODE (x))
>> +		       - INTVAL (XEXP (x, 1)) - bit).to_constant ();
>> +	      x = XEXP (x, 0);
>
> Should mask be shifted by bit here, like it is for the subregs?  E.g.:
>
>   (set (zero_extract:SI R (const_int 2) (const_int 7)))
>
> would currently give a mask of 0x3 and a bit of 7, whereas I think it
> should be a mask of 0x180.  Without that we would only treat the low 8
> bits of R as live, rather than the low 16 bits.

Or I suppose more to the point...

>> +
>> +	      /* We can certainly get (zero_extract (subreg ...)).  The
>> +		 mode of the zero_extract and location should be sufficient
>> +		 and we can just strip the SUBREG.  */
>> +	      if (SUBREG_P (x))
>> +		x = SUBREG_REG (x);
>> +	    }
>> +
>> +	  /* BIT >= 64 indicates something went horribly wrong.  */
>> +	  gcc_assert (bit <= 63);
>> +
>> +	  /* Now handle the actual object that was changed.  */
>> +	  if (REG_P (x))
>> +	    {
>> +	      /* Transfer the appropriate bits from LIVENOW into
>> +		 LIVE_TMP.  */
>> +	      HOST_WIDE_INT rn = REGNO (x);
>> +	      for (HOST_WIDE_INT i = 4 * rn; i < 4 * rn + 4; i++)
>> +		if (bitmap_bit_p (livenow, i))
>> +		  bitmap_set_bit (live_tmp, i);
>> +
>> +	      /* Now clear the bits known written by this instruction.
>> +		 Note that BIT need not be a power of two, consider a
>> +		 ZERO_EXTRACT destination.  */
>> +	      int start = (bit < 8 ? 0 : bit < 16 ? 1 : bit < 32 ? 2 : 3);
>> +	      int end = ((mask & ~0xffffffffULL) ? 4
>> +			 : (mask & 0xffff0000ULL) ? 3
>> +			 : (mask & 0xff00) ? 2 : 1);
>> +	      bitmap_clear_range (livenow, 4 * rn + start, end - start);

...these assumptions don't seem safe for a ZERO_EXTRACT.  Unlike for
the other destinations, IIUC we can only clear liveness bits if the
ZERO_EXTRACT writes to *all* of the bits in the range, rather than
some of them.  The bits that aren't explicitly written to are preserved.

So...

> [...]
> But for now it might be simpler to remove ZERO_EXTRACT from the "else if"
> and leave a FIXME, since continuing will be conservatively correct.

...I think this applies more generally.  It might be safer to handle
ZERO_EXTRACT conservatively for GCC 14 and only try to optimise it in GCC 15.

Thanks,
Richard