On Mon, Nov 5, 2012 at 9:30 AM, Richard Sandiford
<rdsandif...@googlemail.com> wrote:
> This is a case where we had:
>
> (set (reg:HI foo) (plus:HI (reg:HI sp) (const_int X)))
> (clobber (reg:CC FLAGS_REG))
>
> and the splitters decided to convert it to an LEA:
>
> (set (reg:SI foo) (plus:SI (subreg:SI (reg:HI sp) 0) (const_int X)))
>
> But this fails to match, because ix86_address_subreg_operand
> doesn't allow subregs of the stack pointer.
>
> This shows up an inconsistency in the way the generic code handles
> subregs of the stack pointer. Normally we refuse to fold them, even
> after reload, but the final_p case of alter_subreg folds them anyway.
> That's how we ended up with the rather odd 16-bit sp.
>
> However, even if the special alter_subreg case was removed
> (so that we continued to use stack_pointer_rtx itself), we'd have:
>
> (set (reg:HI foo) (plus:HI (subreg:HI (reg:DI sp) 0) (const_int X)))
> (clobber (reg:CC FLAGS_REG))
>
> which would get converted to:
>
> (set (reg:SI foo) (plus:SI (subreg:SI (reg:DI sp) 0) (const_int X)))
>
> and we'd ICE in the same way.
>
> The reason x86 rejects subregs of the stack pointer is this same refusal
> to fold. ix86_print_operand_address tries to simplify a SUBREG to a REG
> and simplify_subreg wouldn't do anything for sp.
>
> simplify_subreg isn't a lot of help at the output stage though.
> If the insn stream contains a subreg that could be simplified but
> hasn't been, then IMO that's a bug. The cases we have to handle here
> are those that can't be simplified (unless we decide at some point that
> all registers must be simplifiable after reload, in which case we shouldn't
> need to handle SUBREGs at all).
>
> As things stand, I think we should be using true_regnum in this case instead.
>
> Tested on x86_64-linux-gnu. OK to install?
Let's ask H.J. to test this change on x32.
> gcc/
> PR target/55204
> * config/i386/i386.c (ix86_address_subreg_operand): Remove stack
> pointer check.
> (print_reg): Use true_regnum rather than REGNO.
> (ix86_print_operand_address): Remove SUBREG handling.
The patch is OK for mainline and 4.7, if it passes H.J.'s tests with
-maddress-mode={short,long} on x32.
> + unsigned int regno = true_regnum (x);
I'd rather see the declaration at the beginning of the function.
Thanks,
Uros.
