tejohnson added a comment. In D60162#1498392 <https://reviews.llvm.org/D60162#1498392>, @tejohnson wrote:
> In D60162#1498288 <https://reviews.llvm.org/D60162#1498288>, @tejohnson wrote: > > > Working on this one again as it previously wasn't causing any issues but > > just now got exposed in multiple places and started causing correctness > > issues. Two questions: > > > > 1. What should be the behavior when merging modules e.g. for LTO. I'm > > thinking of the following, with multiple module flags to specify different > > aspects: a) For the NoBuiltinFuncs I'm thinking of a module flag containing > > the list of strings, with AppendUnique merging behavior, so we get a > > superset in the merged module b) For the flag to disable all builtins, have > > a separate module flag with a Max merge (so we get the most conservative > > behavior) c) For the VectorLibrary, not sure - should it be an Error to > > merge modules with different libraries? > > 2. While we get the above hammered out, would it be ok to submit this one > > (and companion clang change) to fix the bug (then remove when the module > > flags are added)? > > > Weird, phabricator added numbering to all of my bullets, which formatted very > differently than intended. Modified the above a bit to hopefully format > better (2 high level questions, and some subquestions on the first one). I went ahead and implemented the above approach, PTAL. I added test cases for ensuring that the various clang driver flags set up the module flags properly, that merging behaves as expected, and that these options correctly affect LTO backends. Note that this patch now includes both clang and llvm changes. I am going to mark the old clang child patch as obsolete. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60162/new/ https://reviews.llvm.org/D60162 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
