Re: [PATCH/RFC] Avoid invalid subregs during reload, call for testers

[Ian Lance Taylor <iant at google dot com>]

Rask Ingemann Lambertsen <rask@sygehus.dk> writes:

> Index: reload1.c
> ===================================================================
> --- reload1.c	(revision 119451)
> +++ reload1.c	(working copy)
> @@ -5622,10 +5622,11 @@ choose_reload_regs (struct insn_chain *c
>  	      else if (GET_CODE (rld[r].in_reg) == SUBREG
>  		       && REG_P (SUBREG_REG (rld[r].in_reg)))
>  		{
> -		  byte = SUBREG_BYTE (rld[r].in_reg);
>  		  regno = REGNO (SUBREG_REG (rld[r].in_reg));
>  		  if (regno < FIRST_PSEUDO_REGISTER)
>  		    regno = subreg_regno (rld[r].in_reg);
> +		  else
> +		    byte = SUBREG_BYTE (rld[r].in_reg);
>  		  mode = GET_MODE (rld[r].in_reg);
>  		}

This part of the patch is OK.

I'm not convinced that the assert in subeg_regno_offset is correct.
As Joseph has pointed out, this is going to have knock-on effects on
the rest of the compiler.  The call to subreg_regno_offset is not
necessarily incorrect, even if using it would be incorrect.

In particular, without the assert, I don't think the rest of your
patch to reload1.c is needed.  The inherited reload should be
prevented by the call to HARD_REGNO_MODE_OK.  So I would rather
postpone changes like that until we have a clearer idea of how we want
to handle cases like this.

To put this another way, I don't think we should assert unless we
audit every call to subreg_regno_offset and figure out how it should
be handled.

Thanks.

Ian