[PATCH] D74030: [DebugInfo] Avoid generating duplicate llvm.dbg.value

[Alok Kumar Sharma via Phabricator via llvm-commits]

alok marked 9 inline comments as done.
alok added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:1233
 
-/// See if there is a dbg.value intrinsic for DIVar before I.
+/// See if dbg.value intrinsic DVI matches
+static bool isDbgValueMatches(DbgValueInst *DVI, Instruction *I,
----------------
dstenb wrote:
> [...] matches //the given operands.//
> 
> (Or something along those lines).
Thanks. It will be updated in next version.


================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:1234
+/// See if dbg.value intrinsic DVI matches
+static bool isDbgValueMatches(DbgValueInst *DVI, Instruction *I,
+                              DILocalVariable *DIVar, DIExpression *DIExpr,
----------------
dstenb wrote:
> dstenb wrote:
> > Can these pointer variables be made const?
> `isDbgValueMatches` reads a bit awkward to me. Perhaps `isDbgValueMatching`?
Thanks a lot. It shall be updated in next version.


================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:1234-1235
+/// See if dbg.value intrinsic DVI matches
+static bool isDbgValueMatches(DbgValueInst *DVI, Instruction *I,
+                              DILocalVariable *DIVar, DIExpression *DIExpr,
+                              DebugLoc DbgLoc) {
----------------
alok wrote:
> dstenb wrote:
> > dstenb wrote:
> > > Can these pointer variables be made const?
> > `isDbgValueMatches` reads a bit awkward to me. Perhaps `isDbgValueMatching`?
> Thanks a lot. It shall be updated in next version.
Sure. It shall be updated in next version. Thanks for input.


================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:1242
+    if (DVI->getVariable() == DIVar && DVI->getExpression() == DIExpr &&
+        (DVI->getValue() == I->getOperand(0) || DVI->getValue() == I))
+      return true;
----------------
dstenb wrote:
> To make this function more general, perhaps we should make it take a `Value` instead of an `Instruction`, and specify `I` or `I->getOperand(0)` at the call sites?
It shall be updated in next version. Thanks for input.


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

https://reviews.llvm.org/D74030