[PATCH] D81939: [deadargelim] Attach dbg info to the insert/extractvalue instructions

[Vedant Kumar via Phabricator via llvm-commits]

vsk added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp:973
               // return value
               V = ExtractValueInst::Create(NewCB, NewRetIdxs[Ri], "newret",
                                            InsertPt);
----------------
aprantl wrote:
> @vsk: would it be better style to ad-hoc create an IRBuilder with the correct debug location here?
Yes, that seems to be the common idiom. I recommend using `IRBuilder<NoFolder>` to avoid spurious test changes. 


================
Comment at: llvm/test/Transforms/DeadArgElim/multdeadretval.ll:7
 
+; RUN: opt < %s -enable-debugify=synthetic -deadargelim -S 2>&1 \
+; RUN:     | FileCheck %s -check-prefix=DEBUG
----------------
I recommend copying this test, modifying it to include debug info, and dropping the -enable-debugify=synthetic part. This bugfix doesn't need to depend on the debugify original mode patchset. Also, the hardcoded checks for DILocation line numbers will make this test hard to modify, so if we want to check specific synthetic line numbers I think we'd be better served by a dedicated test.


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

https://reviews.llvm.org/D81939