[PATCH] D136320: [Assignment Tracking Analysis][1/*] Add analysis pass core

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando added inline comments.


================
Comment at: llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp:383-384
+  FunctionVarLocsBuilder *FnVarLocs;
+  DenseMap<const BasicBlock *, BlockInfo> LiveIn;
+  DenseMap<const BasicBlock *, BlockInfo> LiveOut;
+
----------------
jmorse wrote:
> Orlando wrote:
> > jmorse wrote:
> > > These translate to maps of maps again, which risks an expensive reallocation. There isn't necessarily anything to do about this if that's not triggered by the workloads that actaully exist.
> > I'm not sure that these ones can be avoided. These are at least initialized to a size equal to the number of basic blocks, down in `AssignmentTrackingLowering::run`.
> Hrrrmmm. I suppose over in LiveDebugValues we have exactly this, but with a SmallVector of DenseMaps where the block number is the index. That's pretty much identical to `LiveIn` / `LiveOut` here. Given this is a dataflow algorithm, random access to blocks is unavoidable, and within those random access to variables, which requires two levels of map.
> 
> That being said, I eventually found it faster to deal with a single variable at a time and trade one locality with another. But again, we should only re-visit this if it turns out that there's a performance loss.
SGTM


================
Comment at: llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp:824
+          dbgs() << "Val, Stack matches Debug program but address is undef\n";);
+      setLocKind(LiveSet, Var, LocKind::Val);
+    } else {
----------------
jmorse wrote:
> Orlando wrote:
> > jmorse wrote:
> > > It looks slightly out of place that the LocKind is recorded as a Val, but then emitDbgValue is used with LocKind::Mem -- I suppose the problem is that I don't know what's meant by elision of the original store. What are the effects of this scenario {LocKind==Val,emitDbgValue(Mem...)}?
> > > It looks slightly out of place that the LocKind is recorded as a Val, but then emitDbgValue is used with LocKind::Mem
> > 
> > It does look slightly out of place. This could be a bug - I'll get back to you on this one.
> (This space intentionally left blank -- to signal to myself that there'll be a response coming back from you at some point).
Updated this (use same LocKind for setLocKind / emitDbgValue here). I am not sure that we need the `undef` check at all though  as this pattern is no longer generated (at least not on purpose). This change results in a `DBG_VALUE $noreg` to lose `DW_OP_deref` from its expression in `mem-loc-frag-fill.ll` in the test patch.


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

https://reviews.llvm.org/D136320