[PATCH] D83738: [Attributor][WIP]Introduce call base context argument pathway.

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:1416
+  /// from call site specific information.
+  void populateArgumentPathwayForFunction(Function *Funct);
+
----------------
Nit: For better or worse we use `F` and `Fn` as a variable name for functions usually.

I read the comment and do not know what happens. We need to improve the wording, maybe elaborate.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:96
+                                   "specific analysis will invalidate."),
+                          cl::init(6));
+
----------------
I'm confused by the description. Is the analysis invaliding values?


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:807
+void Attributor::populateArgumentPathwayForFunction(Function *Funct) {
+  assert(Funct != nullptr);
+
----------------
Nit: Assertions always with messages.

---

Can we have a explanation here or in the header what is happening here. The commit message should also contain more details.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:815
+    Worklist.push_back(V);
+  }
+
----------------
Style: No braces. Use a range loop or part of the constructor (or use append).


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

https://reviews.llvm.org/D83738