[PATCH] D53294: [ThinLTO] Add an option to disable (thin)lto internalization.

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson accepted this revision.
tejohnson added a comment.
This revision is now accepted and ready to land.

LGTM with some minor comment suggestions



================
Comment at: test/LTO/X86/internalize.ll:7
+; Test the enable-lto-internalization option by setting it to false.
+; enable-lto-internalization makes sure internalization does not happen.
+; RUN: llvm-lto %t1.bc -enable-lto-internalization=false -o %t1.save.opt  \
----------------
Either change to "enable-lto-internalization=false makes sure..." or maybe just change to "This make sure..." for simplicity, since you already mention the option name on the prior line.


================
Comment at: test/LTO/X86/internalize.ll:18
+; Test the enable-lto-internalization option by setting it to false.
+; enable-lto-internalization=false makes sure internalization does not happen in runRegularLTO().
+; RUN: llvm-lto2 run %t1.bc -o %t.o -save-temps -enable-lto-internalization=false \
----------------
Same suggestion: "This makes sure..." (will also fix line length which seems too long).


================
Comment at: test/ThinLTO/X86/internalize.ll:7
+; Test the enable-lto-internalization option by setting it to false.
+; enable-lto-internalization makes sure indices are not marked as
+; internallinkage and therefore internalization does not happen.
----------------
Same suggestion as above


================
Comment at: test/ThinLTO/X86/internalize.ll:20
+; Test the enable-lto-internalization option by setting it to false.
+; enable-lto-internalization makes sure indices are not marked as
+; internallinkage and therefore internalization does not happen.
----------------
Same suggestion as above


Repository:
  rL LLVM

https://reviews.llvm.org/D53294