[PATCH] D54997: [DebugInfo] PR39807: Create unknown locations when hoisting to locationless blocks

[Jeremy Morse via Phabricator via llvm-commits]

jmorse marked an inline comment as done.
jmorse added inline comments.


================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:1403
+                           : DILocation::getMergedLocation(I1->getDebugLoc(),
+                                                           I2->getDebugLoc()));
 
----------------
nikic wrote:
> The non-terminator case in L1330 uses `applyMergedLocation()`, which is basically what is done here, minus the attempt to use `InsertPt->getDebugLoc()` first.
> 
> I'm wondering why these two cases are handled differently. While using the debugloc of the insertion point is more correct than using the debugloc of an arbitrary (possibly dead) branch, it still does not seem correct either. Shouldn't we just always use the result of merging debug locs here?
Good question -- my impression is that adopting from the insertion point increases the probability of salvaging earlier locations, which might be optimised out after this transformation. That's not an immensely robust argument though, I'd appreciate additional opinions.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54997/new/

https://reviews.llvm.org/D54997