On Tue, Sep 20, 2011 at 08:19:02AM -0300, Alexandre Oliva wrote:
> > Those need to be emitted uncached
>
> Would you please remind me why that is? The reason totally escapes me.
For NOTE_INSN_VAR_LOCATION, we are caching because we prefer to create
shorter .debug_loc location lists over longer ones (and also to save
memory), if an old location is still valid, we can just extend its range
instead of saying the var now lives somewhere else.
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 is found (and as the numbers show, it decreases
it a lot).
> > 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.
> > 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.
> That's what “stop relying on cselib-implied locs” is about.
>
> > Isn't it too expensive to enter it for all the registers,
> > even when they aren't going to be used for argument passing nor needed
> > explicitly for anything debug info related?
>
> How do you know they're not going to be used? If the register is
> overwritten afterwards, the original expression still serves as an
> alternate location for the VALUE. If we took it from the cselib_val loc
> list, we'd still have it (assuming it's right), but 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, for call arg locations
that resulted in worse debug info though, so I've special cased the
registers using for passing arguments. I guess we could meassure the
memory/hash table size/time requirements of handling all registers here,
or just argument passing ones.
> >> - item = gen_rtx_CONCAT (mode, item, DECL_RTL (dtemp));
> >> + item = gen_rtx_CONCAT (mode, item, DECL_RTL_IF_SET (dtemp));
>
> > Generating CONCAT with NULL second argument?
>
> No, it is always set for dtemp, but DECL_RTL_IF_SET is cheaper because
> it doesn't test whether it's set and call the setter.
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.
Jakub
