[PATCH] D63444: [ThinLTO] Optimize write-only globals out

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

I didn't get very far today, will finish the review tomorrow morning.



================
Comment at: include/llvm/IR/ModuleSummaryIndex.h:210
+  void setReadOnly() {
+    // We expect ro/wo attribute to set only once during
+    // ValueInfo lifetime.
----------------
Is this checked anywhere?  Should the assert be checking that the bit is not yet set?


================
Comment at: include/llvm/IR/ModuleSummaryIndex.h:212
+    // ValueInfo lifetime.
+    assert(isValidAccessSpecifier());
+    RefAndFlags.setInt(RefAndFlags.getInt() | ReadOnly);
----------------
Probably better to check this after it is set below, to catch case of both being set more immediately.


================
Comment at: include/llvm/IR/ModuleSummaryIndex.h:216
+  void setWriteOnly() {
+    assert(isValidAccessSpecifier());
+    RefAndFlags.setInt(RefAndFlags.getInt() | WriteOnly);
----------------
ditto


================
Comment at: include/llvm/IR/ModuleSummaryIndex.h:745
+    // This happens, because attribute propagation occurs on
+    // think link phase.
+    unsigned WriteOnly : 1;
----------------
s/think/thin/


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

https://reviews.llvm.org/D63444