[PATCH] D68633: fix debug info affects output when opt inline

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added a comment.
Herald added a subscriber: ormris.

Looks good, one concern about whether too many debug instructions may get moved written inline



================
Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1842-1843
 
+      // Debuginfo (@llvm.dbg.value) will make different result, skip while allocas scanning
+      while (isa<DbgInfoIntrinsic>(I)) ++I;
+
----------------
Is there a possibility of an unrelated debug instruction being skipped here, and becoming part of the slice moved by lines 1847-1857? Moving dbg.values of arguments to the start of the caller may create a debug use-before-def situation, there could be other problem scenarios too.

Using a debug-instruction filtering iterator (like here [0]) might just do-the-right-thing, I don't know whether feeding one to splice would behave correctly though.

[0] https://github.com/llvm/llvm-project/blob/fdaa74217420729140f1786ea037ac445a724c8e/llvm/lib/Transforms/Utils/SimplifyCFG.cpp#L2592


================
Comment at: llvm/test/Transforms/Inline/inline-with-debuginfo.ll:4-5
+;
+; The purpose of this test is to check that debug info doesn't influence
+; inlining decisions.
+
----------------
The bug report number would be nice too


================
Comment at: llvm/test/Transforms/Inline/inline-with-debuginfo.ll:57-59
+attributes #0 = { nounwind readnone speculatable }
+attributes #2 = { argmemonly nounwind }
+attributes #3 = { nounwind readnone speculatable }
----------------
We generally delete function attributes if they're not necessary


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

https://reviews.llvm.org/D68633