[PATCH] D27804: [SimplifyCFG] Merge debug locations when hoisting an instruction from a then/else branch. NFC.

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added a comment.

Thanks for the review Adrian.



================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:1279
+    // I1 or I2.
     if (!isa<CallInst>(I1) &&  I1->getDebugLoc() != I2->getDebugLoc())
+      I1->setDebugLoc(
----------------
aprantl wrote:
> Wouldn't it make more sense to make getMergedLocation() be an identity operation if the locations are identical?
Yes.

I was chatting with Robert about this. He told me that he plans to send a patch to improve `DILocation::getMergeLocation()` and fix the FIXMEs in `DILocation::canDiscriminate()`. From what I understand, his idea is to use `DILocation::canDiscriminate()` from within `getMergeLocation` to disambiguate cases where two locations are identical (not just same pointers, but also same values).

If it is okay for you, then I would just commit this patch and leave to Rob the task of improving `getMergedLocation()`. What do you think?


https://reviews.llvm.org/D27804