[PATCH] D36233: [ThinLTO] Add FunctionAttrs to ThinLTO index

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

Overall looks good, just a few minor things below. Do you have a follow up dependent patch you can send for review that shows how one of these attributes will be utilized? I'd prefer to have this committed when we have a user for at least one of these new attributes.



================
Comment at: include/llvm/IR/ModuleSummaryIndex.h:305
 
+  FFlags FunFlags;
+
----------------
Needs a comment here as well. Probably just a clone of the one on the structure is fine.


================
Comment at: lib/Bitcode/Reader/BitcodeReader.cpp:5100
     }
-    // FS_PERMODULE: [valueid, flags, instcount, numrefs, numrefs x valueid,
+    // FS_PERMODULE: [valueid, flags, instcount, fflags, numrefs, numrefs x
+    // valueid,
----------------
better to move "numrefs x valueid" to the same line, so that it is formatted consistently (see FS_PERMODULE_PROFILE just below)


================
Comment at: test/Bitcode/thinlto-function-summary-functionattrs.ll:4
+
+; CHECK: <GLOBALVAL_SUMMARY_BLOCK
+; ensure @f is marked readnone
----------------
Can you add a check for noalias as well?


https://reviews.llvm.org/D36233