On Sep 20, 2011, Jakub Jelinek <ja...@redhat.com> wrote:
> For NOTE_INSN_CALL_ARG_LOCATION, the locations aren't location lists, but
> a single location at the point of the call. They are independent of
> all other locations, so any kind of caching only decreases the chance
> that a suitable location
With the proposed patch, we cache full expressions, and we should have
an expression if there's a location for the expression (save for
expression depth limits).
> is found (and as the numbers show, it decreases it a lot).
It decreased because of a bug: many equivalence expressions that used to
be only in the cselib equivalence lists were no longer used with the
proposed patch. I have a fix to bring them back in, but see below.
>> > can't it be postponed to start of vt_emit* phase
>> If my hunch is correct, no. My concern is precisely that the equivalent
>> may cease to hold (say once we cross a val_reset within a loop), but
>> we'll keep on relying on it.
> After vt_initialize, all cselib locations should hold are just some
> expressions containing VALUEs, constants, ENTRY_VALUEs and nothing else,
> all REGs and MEMs are supposed to be flushed. Those are equivalences that
> are always constant, they don't need any kind of resetting and they never
> cease to hold. Say VALUE3 is always VALUE1 + VALUE2.
I realize that. The problem is, I had observed nonsensical equivalences
in dataflow_set loc lists such as (plus (value) (const_int)) in the
location list of the value itself (and a nonzero constant). After much
pondering, I've concluded that this was a symptom of the first round of
dataflow analysis in the initial implementation of VTA, in which we used
union rather than intersection semantics. This is no longer the case,
and I've now convinced myself this can't happen any more, so we can rely
on cselib equivalences, not just for expanding location expressions as
we did before my patch (and will do again in a subsequent version of
it), but also for dataflow set merging (the next algorithm I'm going to
try to optimize).
>> > Can't you use ENUM_BITFIELD (onepart_enum) onepart : 8; instead?
>> I don't think it will then be packed in the same word as n_var_parts.
> enum A { B, C, D };
> struct S { char a; enum A b : 8; char c; char d; };
> int i = sizeof (struct S);
> results in i = 4 for all the cc1 and cc1plus cross compilers on my box I've
> tried, with various ABI options.
Yeah, I'm pretty sure GCC will behave like that on nearly all ABIs, but
that's not mandated by standards. Indeed, IIRC even enum bitfields
aren't mandated by standards. That's why we have ENUM_BITFIELD, after
all! Anyway, since ENUM_BITFIELD was supposed to address this very
issue, I guess it just makes sense for me to go with it ;-)
>> if we stop using cselib locs (like this patch does), if we didn't
>> record the equivalent in the dataflow_set table, we'd lose it.
> They weren't used by var-tracking before at all
Actually, they were, but in a somewhat obscure way: when a
cselib_expand_value_rtx_cb callback returns the VALUE it was asked to
expand, cselib proceeds to expand it using its own equivalence list.
vt_expand_loc_callback would often return it, when it couldn't find an
expansion on its own.
I'll probably make it explicit in vt_expand_loc_callback now, so as to
be able to select the best expansion among all.
> If it is just performance optimization, I'd say there should be
> gcc_checking_assert (DECL_RTL_SET_P (dtemp));
> before it to verify and make it obvious that you aren't expecting it to be
> NULL.
I'm adding DECL_RTL_KNOWN_SET, with a DECL_RTL_SET_P checking_assert,
and using that. How's that?
--
Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/ FSF Latin America board member
Free Software Evangelist Red Hat Brazil Compiler Engineer
