[PATCH] Fix a VTA ICE caused by reload DEBUG_INSN substitution
Richard Sandiford
rdsandiford@googlemail.com
Wed Sep 23 21:54:00 GMT 2009
Jakub Jelinek <jakub@redhat.com> writes:
> On Wed, Sep 23, 2009 at 08:53:19PM +0100, Richard Sandiford wrote:
>> Jakub Jelinek <jakub@redhat.com> writes:
>> > This patch fixes very similar problem to what
>> > http://gcc.gnu.org/ml/gcc-patches/2009-09/msg00779.html
>> > fixed, but this time in reload. If equiv is a constant, and a reg that is
>> > being replaced is insice of e.g. ZERO_EXTEND, things will crash later on, as
>> > (zero_extend:DI (const_int 0)) is invalid.
>> >
>> > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux. Ok for
>> > trunk?
>>
>> Sorry to sound like a broken record, but I'm worried about the
>> growing proliferation of wrap_constants throughout the source.
>> It'd be nice to have some documentation to say where these wrapped
>> constants are valid in the insn stream, and what routines should be
>> prepared to cope with pre-wrapped constants.
>
> In the IL they are supposed to be IMHO just in
> DEBUG_INSNs/NOTE_VAR_LOCATIONs, during the optimization they might appear
> also in passes that use cselib (e.g. DSE). The routines that need to deal
> with those are in simplify-rtx.c, var-tracking.c and dwarf2out.c.
OK, but could you be more specific? E.g.
- Must all simplify-rtx.c routines be prepared to accept (const
(const_int ...)) as a top-level rtx input?
- What about (const (const_int ...))s embedded in other rtx inputs?
Are the simplify-rtx.c routines expected to handle those too?
If not, who is responsible for making sure that the consts are
unwrapped before being used as an operand to another rtx?
- Are the simplify-rtx.c routines allowed/expected to _create_
wrapped constants, or are they only supposed to consume them?
Are they allowed to return a wrapped constant if given a
wrapped constant?
- Since cselib.c and simplify-rtx.c are widely used, who is responsible
for making sure that (const (const_ints ...)) don't leak outside
DEBUG_INSNs and NOTE_VAR_LOCATIONs?
- You mention dse.c as a consumer of cselib, but postreload.c
and the scheduler use it too. Are they expected to handle
(const (const_ints ...))? If not, how do we make sure that
they don't see them, given the new wrapping in cselib.c?
TBH, as per the previous thread, I still don't understand why
cselib_expand_value_rtx_1 has to call wrap_constant (which is
new behaviour that came in with the VTA merge). It's probably
just me being dense, sorry, but a comment might be good.
Hope it doesn't sound like I'm out to be awkward here. ;(
It's just that, as mentioned before, I've suffered the pain of having
CONSTs underspecified in the past. And Kenny and others have suffered
the pain of having subregs underspecified. I'm just worried that adding
this new construct to the insn stream without really defining it is
going to lead to similar problems. Patches like the one for this CR
suggest it isn't entirely obvious. And your previous simplify-rtx.c
patch suggested it wasn't entirely obvious there either, since the patch
differed from simplify_unary in the way it handled the unwrapping.
> Where would you like this to be documented?
rtl.texi would probably be the best place.
Richard
More information about the Gcc-patches
mailing list