[PATCH] D18634: Don't IPO over functions that can be de-refined

[Chandler Carruth via llvm-commits]

chandlerc added inline comments.

================
Comment at: include/llvm/IR/GlobalValue.h:248
@@ -247,3 +247,3 @@
   /// then the code defining it may be replaced by different code.
   static bool mayBeOverridden(LinkageTypes Linkage) {
     return Linkage == WeakAnyLinkage || Linkage == LinkOnceAnyLinkage ||
----------------
I think it would be helpful to rename this to match your exact predicates. If you want to keep some parts, i would make them private. This will nicely ensure the complete audit of call sites.

================
Comment at: include/llvm/IR/GlobalValue.h:289
@@ +288,3 @@
+  /// just isn't the *only* correct implementation.
+  bool mayBeDerefined() const {
+    switch (getLinkage()) {
----------------
I would make this private, as folks should really be calling the predicates below.

================
Comment at: include/llvm/IR/GlobalValue.h:296-298
@@ +295,5 @@
+
+    default:
+      return false;
+    }
+
----------------
This seems wrong. If the function may be overridden, then it necessarily may be overridden by a function which is less refined?

I actually think having these form nicely nested sets is better.

================
Comment at: lib/Transforms/IPO/FunctionAttrs.cpp:493-494
@@ -492,3 +492,4 @@
   for (Function *F : SCCNodes) {
-    // Definitions with weak linkage may be overridden at linktime with
+    // Definitions with non-exact definitions may be overridden at linktime with
     // something that captures pointers, so treat them like declarations.
+    if (!F->hasExactDefinition())
----------------
I would change this to be positive: "We can only infer things using an exact definition."

================
Comment at: lib/Transforms/IPO/FunctionAttrs.cpp:748-749
@@ -747,3 +747,4 @@
 
-    // Definitions with weak linkage may be overridden at linktime, so
+    // Definitions with non-exact definitions may be overridden at linktime, so
     // treat them like declarations.
+    if (!F->hasExactDefinition())
----------------
ditto

================
Comment at: lib/Transforms/IPO/IPConstantPropagation.cpp:164-165
@@ -163,3 +163,4 @@
 
-  // If this function could be overridden later in the link stage, we can't
+  // If this function could be derefined later in the link stage, we can't
   // propagate information about its results into callers.
+  if (!F.isDefinitionExact())
----------------
Use exact terminology here?


http://reviews.llvm.org/D18634