[PATCH] D48676: [Local] replaceAllDbgUsesWith: Update debug values before RAUW

Vedant Kumar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 5 13:25:09 PDT 2018


vsk added a comment.

In https://reviews.llvm.org/D48676#1153467, @bjope wrote:

> FYI: Looks like you have address my earlier comments. So I have no blocking remarks.


Thanks for taking a look, I really appreciate your detailed feedback.

> The addition of DomPoint / DT was interesting (not sure that I have figured out exactly which scenarios that are handled)

The two scenarios are:

1. The replacement value is a Constant, will be inserted at a position which dominates the original instruction, or will be inserted immediately after the original instruction (which is deleted later). In this scenario there's no risk of introducing debug value use-before-def.
2. The original value dominates the position where the replacement value will be inserted. A good example of this is the 'alloca-cast-debuginfo.ll' test. In an intermediate step, InstCombine inserts a temporary cast of an alloca:

  %local = alloca %struct.Foo
  %tmpcast = bitcast %local
  call dbg.declare(%tmpcast, "local")
  <some instructions follow>
  %castOfTmpcast = bitcast %tmpcast

InstCombine recognizes that it can remove 'tmpcast' and simplify 'castOfTmpcast' if it promotes the type of the alloca. Since 'tmpcast' has only one use, we try to save its debug users, anticipating that 'tmpcast' will be deleted. The DomPoint / DT machinery gives us a way to prevent this use-before-def for the unlinked 'simplifiedCast':

  %local = alloca i64
  call dbg.declare(%simplifiedCast, "local")
  <some instructions follow>
  %simplifiedCast = bitcast %local

We either salvage the debug user or delete it. The unit test has examples of both situations.

> I think that we still have the problem with only checking dominance of the moved/replaced value though.
>  As was discussed earlier somewhere (a patch in post RA MachineSink) we also might have the problem that there are other dbg.value intrinsics, referring to the same variable (or an overlapping fragment). And the question is if it is correct to reorder such dbg.value intrinsics, or what to do. Matt wrote a TR about it here https://bugs.llvm.org/show_bug.cgi?id=37897

Doesn't this patch sidestep issues with reordering variable updates, because the updates are now all done in-place?


https://reviews.llvm.org/D48676





More information about the llvm-commits mailing list