[PATCH] D75173: [Transform][MemCpyOpt] Add missing DebugLoc to %tmpbitcast

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

Thanks, this is in good shape. A few minor comments inline.



================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:1306
+  if (MDep->getSource()->getType() != ByValArg->getType()) {
+    BitCastInst* TmpBitCast = new BitCastInst(MDep->getSource(), ByValArg->getType(),
                               "tmpcast", CS.getInstruction());
----------------
nit, we generally ask contributors to clang-format (just) their diffs, e.g. with

https://github.com/llvm-mirror/clang/blob/master/tools/clang-format/clang-format-diff.py

I personally hook this up to git via .gitconfig:

```
[alias]
  fmt = ! git diff -U0 --no-color HEAD~1 | ~/src/llvm-project-master/clang/tools/clang-format/clang-format-diff.py -i -p1
```

& run 'git fmt' before committing.


================
Comment at: llvm/test/Transforms/MemCpyOpt/pr37967.ll:3
+
+; CHECK: CheckFunctionDebugify [MemCpy Optimization]: PASS
+; CHECK-NOT: ERROR: Instruction with empty DebugLoc in function _Z3bar3FooS_RiS_ --  %tmpcast = bitcast i8* %1 to %struct.Foo*
----------------
This check line makes for a nice regression test.


================
Comment at: llvm/test/Transforms/MemCpyOpt/pr37967.ll:4
+; CHECK: CheckFunctionDebugify [MemCpy Optimization]: PASS
+; CHECK-NOT: ERROR: Instruction with empty DebugLoc in function _Z3bar3FooS_RiS_ --  %tmpcast = bitcast i8* %1 to %struct.Foo*
+
----------------
Could you change this to a positive test? e.g.

```
; CHECK-LABEL: define {{.*}} @_Z3bar3FooS_RiS_
; CHECK: [[firstAlloca:%.*]] = alloca %struct.Foo
; CHECK: bitcast i8* [[firstAlloca]] to %struct.Foo*, !dbg
```

As-written, it might stop being a useful test if the structure of the IR emitted by the pass changes slightly (seems likely, as the bitcast is a no-op?), or if (for some reason) the -check-debugify output changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75173