public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Eric Botcazou <ebotcazou@adacore.com>
To: Dehao Chen <dehao@google.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] Reset source location for instructions moved out of its original residing basic block
Date: Thu, 01 Nov 2012 15:04:00 -0000	[thread overview]
Message-ID: <3309385.mAl1npuha0@polaris> (raw)
In-Reply-To: <CAO2gOZVa2oqXnj8q3sM3D7D6fV8xa-GvmgxBe7LRYk2Up6EGBA@mail.gmail.com>

> For optimized code, there are many optimizations that can break
> coverage info. Code motion is one of them. This patch actually tries
> to fix the broken coverage info, as illustrated by the unittest.

No, you seems to be misunderstanding what coverage means.  For coverage to be 
correct, it is necessary that every insn generated for a particular source 
construct be associated with this source construct in the debug info.  If you 
associate an insn with another source construct and it is executed, then 
you'll have the other source construct marked as covered, although it might 
not be actually covered during the execution.

Code motion (as any other optimization passes) doesn't break coverage per se.
For example, there is nothing wrong with hoisting a instruction out of a loop 
and keeping its source location if it is always executed; coverage info will 
be correct after the hoisting.  In more complex cases, it might be necessary 
to clear the source location of the hoisted instruction.

> If we clear the debug info for instructions moved to other BB, is it
> acceptable?

Yes, clearing is acceptable in principle, but should be done with extreme care 
since you drop information, so reorder_insns isn't the appropriate place as 
it's too big a hammer.

FWIW, we have related patchlets in our internal tree, like:

	* loop-invariant.c (move_invariant_reg): Clear the locator of the
	invariant's insn after it has been moved.

	* tree-ssa-loop-im.c (move_computations_stmt): Clear the location and
	block of the invariant's statement after it has been moved.


As for the more general problem of jumpiness in GDB for highly optimized code, 
this should be changed in GDB if your users cannot deal with it.  The debug 
info describes the relationship between the generated code and the source code 
and, at high optimization levels, this relationship is not isomorphic at all.
It's up to the source level debugger to filter out the non-isomorphic part of 
the mapping if it deems desirable to do so.

-- 
Eric Botcazou

  reply	other threads:[~2012-11-01 15:04 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-01  5:03 Dehao Chen
2012-11-01  8:56 ` Eric Botcazou
2012-11-01 14:36   ` Dehao Chen
2012-11-01 15:04     ` Eric Botcazou [this message]
2012-11-01 17:00       ` Dehao Chen
2012-11-01 22:57         ` Ian Lance Taylor
2012-11-01 23:07           ` Xinliang David Li
2012-11-01 23:16             ` Dehao Chen
2012-11-01 23:36               ` Xinliang David Li
2012-11-04 22:26         ` Alexandre Oliva
2012-11-26 14:47           ` Richard Biener

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3309385.mAl1npuha0@polaris \
    --to=ebotcazou@adacore.com \
    --cc=dehao@google.com \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).