On Oct 31, 2017, Jeff Law <l...@redhat.com> wrote:
>> @@ -5691,6 +5691,15 @@ expand_gimple_basic_block (basic_block bb, bool
>> disable_tail_calls)
>> goto delink_debug_stmt;
>> else if (gimple_debug_begin_stmt_p (stmt))
>> val = gen_rtx_DEBUG_MARKER (VOIDmode);
>> + else if (gimple_debug_inline_entry_p (stmt))
>> + {
>> + tree block = gimple_block (stmt);
>> +
>> + if (block)
>> + val = gen_rtx_DEBUG_MARKER (BLKmode);
>> + else
>> + goto delink_debug_stmt;
>> + }
>> else
>> gcc_unreachable ();
> So again, I haven't looked at prior patches. It seems to me like we're
> abusing the mode of the debug marker rtx here.
It is still abuse if it's documented in INSN_DEBUG_MARKER_KIND and
rtl.texi? :-)
We need some way to tell the (currently) two kinds of markers apart. I
didn't want yet another rtl code that would have to be added to cases
all over; the distinction between markers matters at only very few
points in the compiler. I considered adding an operand to the debug
marker, to reference a const_int that could tell them apart with room
for growth to other kinds of markers, or using any of the flag bits, but
the mode was the most visibly unused mandatory rtl field in debug
markers, so I went for it.
Changing that would make for a very localized patch, touching only this
creation point, the macro that tells the kinds apart, and the
documentation, so if you'd rather have something else, I'll be glad to
comply.
>> +/* Check whether BLOCK, a lexical block, is nested within OUTER, or is
>> + OUTER itself. */
>> +static bool
>> +block_within_block_p (tree block, tree outer)
>> +{
>> + if (block == outer)
>> + return true;
>> +
>> + /* Quickly check that OUTER is up BLOCK's supercontext chain. */
>> + for (tree context = BLOCK_SUPERCONTEXT (block);
>> + context != outer;
>> + context = BLOCK_SUPERCONTEXT (context))
>> + if (!context || TREE_CODE (context) != BLOCK)
>> + return false;
>> +
>> + /* Now check that each block is actually referenced by its
>> + parent. */
> So this seemed reasonable up to here. Based on the name and function
> comment, I'd think at this point you'd be able to return true. We found
> OUTER in BLOCK's supercontext chain.
The part you quoted looks at only at child-to-parent links; the other
part looks at parent-to-children links, to make sure they match. The
function is a consistency check, used only in a checking assert. It's
intended to check both links, that the parent is reachable by the child,
and that the child is reachable by the parent. At some point, I had
blocks that had been disconnected and discarded from the lexical block
tree, but that remained pointed-to by markers; they still pointed to
their parents, but their parents no longer pointed to them.
> But you've got another blob of code. So it seems like you're check more
> than what the name/function comment imply.
I guess I should expand the comments ;-)
Will do...
--
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
