[llvm] r237237 - [DebugInfo] Debug locations for constant SD nodes

Sergey Dmitrouk sdmitrouk at accesssoftek.com
Thu Jun 4 13:59:18 PDT 2015


On Thu, Jun 04, 2015 at 11:02:00AM -0700, David Blaikie wrote:
>    Looks good - please commit when ready.

Commited in r239089.

>    Optional thoughts you can roll into the commit or we can handle in post
>    commit:
> 
>    * The CHECK lines might be a bit more verbose/selectively chosen to be
>    more self documenting (perhaps CHECK for a .loc of line 9, then CHECK-NOT
>    for a .loc, then CHECK for the movl $1 - essentially saying "ensure that
>    the constant 1 is attributed to line 9")

It's not that accurate, because we just remove debug location instead of
setting the correct one.  I'll probably at least try to get correct locations
in this case, but it will lead to duplicating constants (I'd expect it have
small impact though).

>    * You could drop the "getDebugLoc" from the condition you're adding, since
>    it doesn't matter whether it has one or not if we're just removing it
>    anyway
>    * The comment's english is a bit confusing. Specifically in the last
>    clause of the first sentence. Maybe: "Remove the debug location from the
>    node as the node is about to be used in a location which may differ from
>    the original debug location" ? I'm not sure if that's accurate or not,
>    though.

Changes were updated according to your suggestions.

Thanks,
Sergey



More information about the llvm-commits mailing list