[PATCH] Ephemeral values for LLVM invariants

Chandler Carruth chandlerc at gmail.com
Thu Jul 17 11:27:05 PDT 2014


================
Comment at: include/llvm/Analysis/CodeMetrics.h:96
@@ +95,3 @@
+  static void collectEphemeralValues(const BasicBlock *BB,
+                                     DenseSet<const Value *> &EphValues);
+
----------------
Use a SmallPtrSetImpl<const Value*>?

================
Comment at: lib/Analysis/CodeMetrics.cpp:55
@@ +54,3 @@
+                                    DenseSet<const Value *> &EphValues) {
+  DenseSet<const Value *> Visited;
+
----------------
SmallPtrSet is significantly more efficient than DenseSet, please use it everywhere.

================
Comment at: lib/Analysis/CodeMetrics.cpp:70
@@ +69,3 @@
+
+    if (!FoundNEUse) {
+      EphValues.insert(V);
----------------
nit: invert and use 'continue' to reduce indentation

================
Comment at: lib/Analysis/CodeMetrics.cpp:75-76
@@ +74,4 @@
+      if (const User *U = dyn_cast<User>(V))
+        for (User::const_op_iterator J = U->op_begin(), JE = U->op_end();
+             J != JE; ++J) {
+          if (isSafeToSpeculativelyExecute(*J))
----------------
I think we have an ->operands() that will let you write this as a range loop. Even if not, auto would help.

================
Comment at: lib/Analysis/CodeMetrics.cpp:77
@@ +76,3 @@
+             J != JE; ++J) {
+          if (isSafeToSpeculativelyExecute(*J))
+            WorkSet.push_back(*J);
----------------
Why being "ephemeral" is equivalent to being safe to speculate isn't immediately obvious to me. might be nice to add a comment at least ...

================
Comment at: lib/Analysis/IPA/InlineCost.cpp:1106
@@ +1105,3 @@
+  DenseSet<const Value *> EphValues;
+  CodeMetrics::collectEphemeralValues(&F, EphValues);
+
----------------
Oof, so, we can't do it this way in the inline cost analysis for terrible reasons.

The reason why inline cost analysis ends up duplicating so much code from code metrics is because an essential property of inline cost analysis is that the amount of work done should be bounded by the constant threshold, not by the size of the function (or basic block). This is important because we may end up doing a linear number of calls to compute the inline cost where the linear factor would go quadratic if we do a linear amount of work here. =/

This is particularly problematic for ephemeral values because their very nature is defined backwards. I'm happy with this as an initial implementation but it should have a "FIXME" to revisit this before we start emitting them really heavily.

http://reviews.llvm.org/D180






More information about the llvm-commits mailing list