[PATCH] D16169: [Utils] Fix incorrect dbg.declare store conversion

[Adrian Prantl via llvm-commits]

aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.

Good catch! Looks pretty obvious to me. Some minor comments inline.


================
Comment at: lib/Transforms/Utils/Local.cpp:1133
@@ -1133,2 +1133,2 @@
     // the stack slot (and at a lexical-scope granularity). Later
     // passes will attempt to elide the stack slot.
----------------
Please add more/full context to diffs uploaded to phabricator; it makes reviewing easier.

================
Comment at: lib/Transforms/Utils/Local.cpp:1138
@@ +1137,3 @@
+        User *U = AIUse.getUser();
+        if (StoreInst *SI = dyn_cast<StoreInst>(U)) {
+          if (AIUse.getOperandNo() == 1)
----------------
Does turning this if cascade into a switch() make it any more readable?

================
Comment at: test/Transforms/Util/store-first-op.ll:11
@@ +10,3 @@
+; %getU is a store TO %getU. There are valid reasons to have an llvm.dbg.value here, but if the pass
+; pass is changed to emit such, a more specific check should be added to make sure that the llvm.dbg.value
+; is correct.
----------------
pass pass

================
Comment at: test/Transforms/Util/store-first-op.ll:13
@@ +12,3 @@
+; is correct.
+; CHECK-NOT: @llvm.dbg.value(metadata %foo* %getU
+  call void @llvm.dbg.declare(metadata %foo* %getU, metadata !3, metadata !6), !dbg !7
----------------
I would still add a positive check — this way we'll be notified when the testcase needs to be updated.


Repository:
  rL LLVM

http://reviews.llvm.org/D16169