[SFN+LVU+IEPM v4 9/9] [IEPM] Introduce inline entry point markers

[Alexandre Oliva]

On Dec 21, 2017, Jeff Law <law@redhat.com> wrote:

> On 12/11/2017 07:54 PM, Alexandre Oliva wrote:
>> +	      ASM_GENERATE_INTERNAL_LABEL (label, "LVU", ied->view);
>> +	      /* FIXME: this will resolve to a small number.  Could we
>> +		 possibly emit smaller data?  Ideally we'd emit a
>> +		 uleb128, but that would make the size of DIEs
>> +		 impossible for the compiler to compute, since it's
>> +		 the assembler that computes the value of the view
>> +		 label in this case.  Ideally, we'd have a single form
>> +		 encompassing both the address and the view, and
>> +		 indirecting them through a table might make things
>> +		 easier, but even that would be more wasteful,
>> +		 space-wise, than what we have now.  */
>> +	      add_AT_lbl_id (die, DW_AT_GNU_entry_view, label);

> Do you have a sense of whether or not this really matters in practice?
> how much bigger do we get due when we enable LVU?

It's more of a matter of general design principle.  DWARF strives to be
compact, and outputting a very-likely small constant with most bits
known to be zero is kind of against the rules.

LVU proper doesn't run into this because the location view numbers are
emitted as uleb128 constants in location list sections, never in the
main debug section.  How much it really grows depends on the
representation: DWARF>5 loclists only get additional entries when at
least one view number in a range pair is nonzero, and I have a sense
that most view numbers in loclists are zero; for DWARF[2,5], we emit
list of pairs corresponding to the ranges in each entry in the actual
loclist, so we spend at least two bytes for both views, plus the pointer
to the locviewlist in an additional attribute.  Considering each range
amounts to a pair of pointers plus at least two bytes for the location
expression, and that each range gets a corresponding pair of views, and
assuming all views will fit in a single uleb128 byte (128+ views at the
same PC are very unlikely), the loclist section would grow by at most
20% with 32-bit pointers, and about half that much with 64-bit pointers.
In reality, it ought to be a lot less than that, since location
expressions's taking up a single byte (plus another byte representing
the size) are common but hardly a majority of the cases.

>> +  /* Sanity check the block tree.  This would catch a case in which
>> +     BLOCK got removed from the tree reachable from the outermost
>> +     lexical block, but got retained in markers.  It would still link
>> +     back to its parents, but some ancestor would be missing a link
>> +     down the path to the sub BLOCK.  If the block got removed, its
>> +     BLOCK_NUMBER will not be a usable value.  */
>> +  gcc_checking_assert (block_within_block_p (block,
>> +					     DECL_INITIAL
>> +					     (current_function_decl),
>> +					     true));
>> +
>> +  gcc_assert (inlined_function_outer_scope_p (block));
>> +  gcc_assert (!BLOCK_DIE (block));
>> +
>> +  /* If we can't represent it, don't bother.  */
>> +  if (!(dwarf_version >= 3 || !dwarf_strict))
>> +    return;
> Consider moving this check earlier.  I don't think it's a hard
> requirement, so if you put it after the asserts for a reason, then leave
> it has is.

The reason I put the check after the asserts was that I wanted to catch
messed up block trees even if we wouldn't emit the entry point after
all.  Now, considering the block tree messing up was figured out, I
guess moving the test earlier and saving the cycles if we're not
emitting the annotation makes sense.  Will do.

> Generally I think this is fine (it's much simpler than the dwarf2 bits
> of the LVU patch.

Yeah.  I wonder if there's anything I can do, now that I'm back from
vacations, to help get the LVU patch reviewed.  I suppose it might still
be eligible for GCC 8, considering it was posted even before stage3, let
alone stage4, and considering it only deals with debug info (mainly
location lists), but then...  unlike the SFN stuff, the additional
information it outputs will only make a difference once debug info
consumers learn to use it.  So maybe we could afford to leave it out, or
to bring it in but disabled.

Thoughts?  Does it make sense at this point for me to pester ^W entice
some review to have a look at it?

Thanks,

-- 
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 Brasil GNU Toolchain Engineer