[PATCH] D64595: [Debuginfo][SROA] Need to handle dbg.value in SROA pass.

[Alexey Lapshin via Phabricator via llvm-commits]

avl marked 3 inline comments as done.
avl added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/SROA.cpp:4250
+
+void findDeclarations(SmallVectorImpl<DbgVariableIntrinsic *> &Declarations,
+                      Value *V) {
----------------
vsk wrote:
> Why not filter the result from llvm::findDbgUsers?
In that case size of resulting container would be bigger. If that is not a problem - I would change to llvm::findDbgUsers.


================
Comment at: llvm/lib/Transforms/Scalar/SROA.cpp:4379
+
+  for (auto Decl : Declarations) {
+
----------------
vsk wrote:
> Iterating over a DenseSet will give non-deterministic results. You need a vector-like container (or perhaps a SetVector) here.
It looks like vector-like container already used here - SmallVector.


================
Comment at: llvm/lib/Transforms/Scalar/SROA.cpp:4392
       // expression if there is only one partition.
       auto *FragmentExpr = Expr;
+
----------------
vsk wrote:
> I'm not sure this fragment expr is valid for, say, dbg.values with OP_deref attached. I think this would result in a dbg.declare with an OP_deref, which doesn't seem to make sense. Currently you're uniquing debug intrinsics by variable name, so you might not hit the issue right away, but eventually it'll pop up.
> 
> Perhaps it'd help to start with simple expressions, i.e. empty expressions, and then incrementally build on that + add tests.
> 
> 
Thanks, I would change to empty expression then. 


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

https://reviews.llvm.org/D64595