[PATCH] D29668: Elide argument copies during instruction selection

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:8057
+    auto Iter = StaticAllocas.insert({AI, Unknown});
+    return &Iter.first->second;
+  };
----------------
rnk wrote:
> hans wrote:
> > I might have gone with just `return &StaticAllocas[AI]`
> This is C++! We can't do two hash lookups. :)
Elaborating on this, I think that it would be less clear that on first use, a static alloca starts in the "unknown" state.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:8047
+  enum StaticAllocaInfo { Unknown, Clobbered, Elidable };
+  DenseMap<const AllocaInst *, StaticAllocaInfo> StaticAllocas;
+
----------------
chandlerc wrote:
> SmallDenseMap? Seems like we can predict common upper bounds and this is a leaf-y function, so can avoid most memory allocation here.
Done, but I don't think the signals are that good.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:8077
+      for (const Use &U : I.operands()) {
+        if (StaticAllocaInfo *Info = GetInfoIfStaticAlloca(U))
+          *Info = StaticAllocaInfo::Clobbered;
----------------
chandlerc wrote:
> For a sequence of N casts of an alloca, doesn't this do O(N^2) work?
> 
> Maybe make GetInfoIfStaticAlloca fully memoize each Value?
Do you think the memory use of memoization is worth optimizing for long chains of casts that mid-level optimizations usually remove?


https://reviews.llvm.org/D29668