Mehdi Amini via llvm-dev
2015-Sep-16 17:09 UTC
[llvm-dev] RFC: LTO should use -disable-llvm-verifier
> On Sep 16, 2015, at 9:45 AM, Teresa Johnson via llvm-dev <llvm-dev at lists.llvm.org> wrote: > > On Wed, Sep 16, 2015 at 7:47 AM, Teresa Johnson <tejohnson at google.com> wrote: >> On Tue, Sep 15, 2015 at 3:31 PM, Duncan P. N. Exon Smith via llvm-dev >> <llvm-dev at lists.llvm.org> wrote: >>> >>>> On 2015-Sep-02, at 19:31, Peter Collingbourne <peter at pcc.me.uk> wrote: >>>> >>>> On Thu, Sep 03, 2015 at 01:10:42AM +0000, Eric Christopher wrote: >>>>> On Tue, Sep 1, 2015 at 10:43 AM Duncan P. N. Exon Smith < >>>>> dexonsmith at apple.com> wrote: >>>>> >>>>>> >>>>>>> On 2015-Aug-31, at 18:09, Eric Christopher <echristo at gmail.com> wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Mon, Aug 31, 2015 at 5:50 PM Duncan P. N. Exon Smith < >>>>>> dexonsmith at apple.com> wrote: >>>>>>> >>>>>>>> On 2015-Aug-31, at 12:21, Eric Christopher <echristo at gmail.com> wrote: >>>>>>>> Yep. This is where I was going :) >>>>>>> >>>>>>> Glad I found consensus, but I want to double-check that this makes >>>>>>> sense to add to the driver. I didn't quite think through the >>>>>>> implications myself. >>>>>>> >>>>>>> Since the driver doesn't know if there's any bitcode, or if LTO is >>>>>>> going to be invoked, it seems like I'll have to change the noasserts >>>>>>> driver to *always* pass the option to the linker just in case we are >>>>>>> doing LTO. Is this reasonable? >>>>>>> >>>>>>> Also, I realized that passing `-mllvm -disable-llvm-verifier` to ld64 >>>>>>> is redundant... so I'm thinking `-mllvm -disable-verify`. Make >>>>>>> sense? >>>>>>> >>>>>>> *sigh* Reasons to hate the driver interface again... >>>>>>> >>>>>>> I guess this is ok. Could possibly add it to the existing terrible >>>>>> "enable this pass" interface on liblto as well. >>>>>> >>>>>> The linker doesn't know whether clang was built with asserts, though. >>>>>> >>>>>> We could just make it implicit: move the decision to libLTO itself. Given >>>>>> that clang and libLTO.dylib are different executables anyway -- and you >>>>>> might be interposing an asserts libLTO.dylib to use with an installed clang >>>>>> -- maybe it's even better? >>>>>> >>>>>> >>>>> *nod* We could do that. Seems better than the alternative. >>>> >>>> +1 >>> >>> Finally got back to this. Done in r247729. >>> >>> I didn't modify gold-plugin.cpp, as I don't have a good way to test it, >>> but someone else should be able to do it pretty easily. >> >> I can do this for gold (presumably also controlled via an option, but >> set default based on NDEBUG). >> >> Couple questions: >> - For your patch the default is set based on NDEBUG for lto.cpp, but >> in llvm-lto it always defaults to false. Is that intentional? > > After writing a test for my gold-plugin changes, I think I know the > answer to the above question. It is presumably because you don't want > the behavior of new test ./test/LTO/X86/disable-verify.ll which uses > llvm-lto to change based on whether the compiler is built NDEBUG or > not, since you are also testing the default behavior without > -disable-verify in this test. > > Since gold-plugin isn't a testing tool, I think we do want the default > controlled by NDEBUG. So presumably my new gold-plugin-based test > cannot test the default behavior, just the behavior with the new > disable-verify plugin option.Note sure if it would help but you can add “REQUIRE: assert” in a test to only run it when NDEBUG is enabled. There is no “REQUIRE: noassert” though. — Mehdi> > Teresa > >> - You mentioned that the verifier was currently being run 3 times: (1) >> after parsing, (2) at the beginning of the optimization pipeline, and >> (3) at the end of it. It looks to me like (1) is done via the >> createVerifierPass() added in >> LTOCodeGenerator::applyScopeRestrictions(). However, gold does not use >> LTOCodeGenerator, and I don't see it explicitly adding an initial >> createVerifierPass. So it looks like for gold it is only being called >> twice (beginning of optimization pipeline and at the end). So I think >> for gold I need to leave VerifyInput on the pass manager builder set >> to true unconditionally in order to get an initial round of input >> verification. Does that sound right? >> >> Teresa >> >>> _______________________________________________ >>> LLVM Developers mailing list >>> llvm-dev at lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> >> >> >> -- >> Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413 > > > > -- > Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413 > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Teresa Johnson via llvm-dev
2015-Sep-16 17:39 UTC
[llvm-dev] RFC: LTO should use -disable-llvm-verifier
On Wed, Sep 16, 2015 at 10:09 AM, Mehdi Amini <mehdi.amini at apple.com> wrote:> >> On Sep 16, 2015, at 9:45 AM, Teresa Johnson via llvm-dev <llvm-dev at lists.llvm.org> wrote: >> >> On Wed, Sep 16, 2015 at 7:47 AM, Teresa Johnson <tejohnson at google.com> wrote: >>> On Tue, Sep 15, 2015 at 3:31 PM, Duncan P. N. Exon Smith via llvm-dev >>> <llvm-dev at lists.llvm.org> wrote: >>>> >>>>> On 2015-Sep-02, at 19:31, Peter Collingbourne <peter at pcc.me.uk> wrote: >>>>> >>>>> On Thu, Sep 03, 2015 at 01:10:42AM +0000, Eric Christopher wrote: >>>>>> On Tue, Sep 1, 2015 at 10:43 AM Duncan P. N. Exon Smith < >>>>>> dexonsmith at apple.com> wrote: >>>>>> >>>>>>> >>>>>>>> On 2015-Aug-31, at 18:09, Eric Christopher <echristo at gmail.com> wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On Mon, Aug 31, 2015 at 5:50 PM Duncan P. N. Exon Smith < >>>>>>> dexonsmith at apple.com> wrote: >>>>>>>> >>>>>>>>> On 2015-Aug-31, at 12:21, Eric Christopher <echristo at gmail.com> wrote: >>>>>>>>> Yep. This is where I was going :) >>>>>>>> >>>>>>>> Glad I found consensus, but I want to double-check that this makes >>>>>>>> sense to add to the driver. I didn't quite think through the >>>>>>>> implications myself. >>>>>>>> >>>>>>>> Since the driver doesn't know if there's any bitcode, or if LTO is >>>>>>>> going to be invoked, it seems like I'll have to change the noasserts >>>>>>>> driver to *always* pass the option to the linker just in case we are >>>>>>>> doing LTO. Is this reasonable? >>>>>>>> >>>>>>>> Also, I realized that passing `-mllvm -disable-llvm-verifier` to ld64 >>>>>>>> is redundant... so I'm thinking `-mllvm -disable-verify`. Make >>>>>>>> sense? >>>>>>>> >>>>>>>> *sigh* Reasons to hate the driver interface again... >>>>>>>> >>>>>>>> I guess this is ok. Could possibly add it to the existing terrible >>>>>>> "enable this pass" interface on liblto as well. >>>>>>> >>>>>>> The linker doesn't know whether clang was built with asserts, though. >>>>>>> >>>>>>> We could just make it implicit: move the decision to libLTO itself. Given >>>>>>> that clang and libLTO.dylib are different executables anyway -- and you >>>>>>> might be interposing an asserts libLTO.dylib to use with an installed clang >>>>>>> -- maybe it's even better? >>>>>>> >>>>>>> >>>>>> *nod* We could do that. Seems better than the alternative. >>>>> >>>>> +1 >>>> >>>> Finally got back to this. Done in r247729. >>>> >>>> I didn't modify gold-plugin.cpp, as I don't have a good way to test it, >>>> but someone else should be able to do it pretty easily. >>> >>> I can do this for gold (presumably also controlled via an option, but >>> set default based on NDEBUG). >>> >>> Couple questions: >>> - For your patch the default is set based on NDEBUG for lto.cpp, but >>> in llvm-lto it always defaults to false. Is that intentional? >> >> After writing a test for my gold-plugin changes, I think I know the >> answer to the above question. It is presumably because you don't want >> the behavior of new test ./test/LTO/X86/disable-verify.ll which uses >> llvm-lto to change based on whether the compiler is built NDEBUG or >> not, since you are also testing the default behavior without >> -disable-verify in this test. >> >> Since gold-plugin isn't a testing tool, I think we do want the default >> controlled by NDEBUG. So presumably my new gold-plugin-based test >> cannot test the default behavior, just the behavior with the new >> disable-verify plugin option. > > Note sure if it would help but you can add “REQUIRE: assert” in a test to only run it when NDEBUG is enabled.Wouldn't asserts be off generally in NDEBUG mode? And if so does the "assert" check here always imply !NDEBUG? If so, that would do the trick. Also, is there documentation on the REQUIRE values? I couldn't find any under http://llvm.org/docs/TestingGuide.html Thanks, Teresa> > There is no “REQUIRE: noassert” though. > > > — > Mehdi > > > >> >> Teresa >> >>> - You mentioned that the verifier was currently being run 3 times: (1) >>> after parsing, (2) at the beginning of the optimization pipeline, and >>> (3) at the end of it. It looks to me like (1) is done via the >>> createVerifierPass() added in >>> LTOCodeGenerator::applyScopeRestrictions(). However, gold does not use >>> LTOCodeGenerator, and I don't see it explicitly adding an initial >>> createVerifierPass. So it looks like for gold it is only being called >>> twice (beginning of optimization pipeline and at the end). So I think >>> for gold I need to leave VerifyInput on the pass manager builder set >>> to true unconditionally in order to get an initial round of input >>> verification. Does that sound right? >>> >>> Teresa >>> >>>> _______________________________________________ >>>> LLVM Developers mailing list >>>> llvm-dev at lists.llvm.org >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>> >>> >>> >>> -- >>> Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413 >> >> >> >> -- >> Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413 >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >-- Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413
Duncan P. N. Exon Smith via llvm-dev
2015-Sep-16 17:46 UTC
[llvm-dev] RFC: LTO should use -disable-llvm-verifier
> On 2015-Sep-16, at 10:09, Mehdi Amini <mehdi.amini at apple.com> wrote: > >> >> On Sep 16, 2015, at 9:45 AM, Teresa Johnson via llvm-dev <llvm-dev at lists.llvm.org> wrote: >> >> On Wed, Sep 16, 2015 at 7:47 AM, Teresa Johnson <tejohnson at google.com> wrote: >>> On Tue, Sep 15, 2015 at 3:31 PM, Duncan P. N. Exon Smith via llvm-dev >>> <llvm-dev at lists.llvm.org> wrote: >>>> >>>>> On 2015-Sep-02, at 19:31, Peter Collingbourne <peter at pcc.me.uk> wrote: >>>>> >>>>> On Thu, Sep 03, 2015 at 01:10:42AM +0000, Eric Christopher wrote: >>>>>> On Tue, Sep 1, 2015 at 10:43 AM Duncan P. N. Exon Smith < >>>>>> dexonsmith at apple.com> wrote: >>>>>> >>>>>>> >>>>>>>> On 2015-Aug-31, at 18:09, Eric Christopher <echristo at gmail.com> wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On Mon, Aug 31, 2015 at 5:50 PM Duncan P. N. Exon Smith < >>>>>>> dexonsmith at apple.com> wrote: >>>>>>>> >>>>>>>>> On 2015-Aug-31, at 12:21, Eric Christopher <echristo at gmail.com> wrote: >>>>>>>>> Yep. This is where I was going :) >>>>>>>> >>>>>>>> Glad I found consensus, but I want to double-check that this makes >>>>>>>> sense to add to the driver. I didn't quite think through the >>>>>>>> implications myself. >>>>>>>> >>>>>>>> Since the driver doesn't know if there's any bitcode, or if LTO is >>>>>>>> going to be invoked, it seems like I'll have to change the noasserts >>>>>>>> driver to *always* pass the option to the linker just in case we are >>>>>>>> doing LTO. Is this reasonable? >>>>>>>> >>>>>>>> Also, I realized that passing `-mllvm -disable-llvm-verifier` to ld64 >>>>>>>> is redundant... so I'm thinking `-mllvm -disable-verify`. Make >>>>>>>> sense? >>>>>>>> >>>>>>>> *sigh* Reasons to hate the driver interface again... >>>>>>>> >>>>>>>> I guess this is ok. Could possibly add it to the existing terrible >>>>>>> "enable this pass" interface on liblto as well. >>>>>>> >>>>>>> The linker doesn't know whether clang was built with asserts, though. >>>>>>> >>>>>>> We could just make it implicit: move the decision to libLTO itself. Given >>>>>>> that clang and libLTO.dylib are different executables anyway -- and you >>>>>>> might be interposing an asserts libLTO.dylib to use with an installed clang >>>>>>> -- maybe it's even better? >>>>>>> >>>>>>> >>>>>> *nod* We could do that. Seems better than the alternative. >>>>> >>>>> +1 >>>> >>>> Finally got back to this. Done in r247729. >>>> >>>> I didn't modify gold-plugin.cpp, as I don't have a good way to test it, >>>> but someone else should be able to do it pretty easily. >>> >>> I can do this for gold (presumably also controlled via an option, but >>> set default based on NDEBUG). >>> >>> Couple questions: >>> - For your patch the default is set based on NDEBUG for lto.cpp, but >>> in llvm-lto it always defaults to false. Is that intentional? >> >> After writing a test for my gold-plugin changes, I think I know the >> answer to the above question. It is presumably because you don't want >> the behavior of new test ./test/LTO/X86/disable-verify.ll which uses >> llvm-lto to change based on whether the compiler is built NDEBUG or >> not, since you are also testing the default behavior without >> -disable-verify in this test. >> >> Since gold-plugin isn't a testing tool, I think we do want the default >> controlled by NDEBUG. So presumably my new gold-plugin-based test >> cannot test the default behavior, just the behavior with the new >> disable-verify plugin option. > > Note sure if it would help but you can add “REQUIRE: assert” in a test to only run it when NDEBUG is enabled. > > There is no “REQUIRE: noassert” though.It would be possible, but I like that llvm-lto (as a testing tool) behaves consistently regardless of assertions.> — > Mehdi > > > >> >> Teresa >> >>> - You mentioned that the verifier was currently being run 3 times: (1) >>> after parsing, (2) at the beginning of the optimization pipeline, and >>> (3) at the end of it. It looks to me like (1) is done via the >>> createVerifierPass() added in >>> LTOCodeGenerator::applyScopeRestrictions(). However, gold does not use >>> LTOCodeGenerator, and I don't see it explicitly adding an initial >>> createVerifierPass. So it looks like for gold it is only being called >>> twice (beginning of optimization pipeline and at the end). So I think >>> for gold I need to leave VerifyInput on the pass manager builder set >>> to true unconditionally in order to get an initial round of input >>> verification. Does that sound right? >>> >>> Teresa >>> >>>> _______________________________________________ >>>> LLVM Developers mailing list >>>> llvm-dev at lists.llvm.org >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>> >>> >>> >>> -- >>> Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413 >> >> >> >> -- >> Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413 >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Duncan P. N. Exon Smith via llvm-dev
2015-Sep-16 17:47 UTC
[llvm-dev] RFC: LTO should use -disable-llvm-verifier
> On 2015-Sep-16, at 10:39, Teresa Johnson <tejohnson at google.com> wrote: > > > Also, is there documentation on the REQUIRE values? I couldn't find > any under http://llvm.org/docs/TestingGuide.html >I've never found documentation. I usually try to read through the lit configs or do `git grep`s.
Mehdi Amini via llvm-dev
2015-Sep-16 17:50 UTC
[llvm-dev] RFC: LTO should use -disable-llvm-verifier
> On Sep 16, 2015, at 10:39 AM, Teresa Johnson <tejohnson at google.com> wrote: > > On Wed, Sep 16, 2015 at 10:09 AM, Mehdi Amini <mehdi.amini at apple.com> wrote: >> >>> On Sep 16, 2015, at 9:45 AM, Teresa Johnson via llvm-dev <llvm-dev at lists.llvm.org> wrote: >>> >>> On Wed, Sep 16, 2015 at 7:47 AM, Teresa Johnson <tejohnson at google.com> wrote: >>>> On Tue, Sep 15, 2015 at 3:31 PM, Duncan P. N. Exon Smith via llvm-dev >>>> <llvm-dev at lists.llvm.org> wrote: >>>>> >>>>>> On 2015-Sep-02, at 19:31, Peter Collingbourne <peter at pcc.me.uk> wrote: >>>>>> >>>>>> On Thu, Sep 03, 2015 at 01:10:42AM +0000, Eric Christopher wrote: >>>>>>> On Tue, Sep 1, 2015 at 10:43 AM Duncan P. N. Exon Smith < >>>>>>> dexonsmith at apple.com> wrote: >>>>>>> >>>>>>>> >>>>>>>>> On 2015-Aug-31, at 18:09, Eric Christopher <echristo at gmail.com> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> On Mon, Aug 31, 2015 at 5:50 PM Duncan P. N. Exon Smith < >>>>>>>> dexonsmith at apple.com> wrote: >>>>>>>>> >>>>>>>>>> On 2015-Aug-31, at 12:21, Eric Christopher <echristo at gmail.com> wrote: >>>>>>>>>> Yep. This is where I was going :) >>>>>>>>> >>>>>>>>> Glad I found consensus, but I want to double-check that this makes >>>>>>>>> sense to add to the driver. I didn't quite think through the >>>>>>>>> implications myself. >>>>>>>>> >>>>>>>>> Since the driver doesn't know if there's any bitcode, or if LTO is >>>>>>>>> going to be invoked, it seems like I'll have to change the noasserts >>>>>>>>> driver to *always* pass the option to the linker just in case we are >>>>>>>>> doing LTO. Is this reasonable? >>>>>>>>> >>>>>>>>> Also, I realized that passing `-mllvm -disable-llvm-verifier` to ld64 >>>>>>>>> is redundant... so I'm thinking `-mllvm -disable-verify`. Make >>>>>>>>> sense? >>>>>>>>> >>>>>>>>> *sigh* Reasons to hate the driver interface again... >>>>>>>>> >>>>>>>>> I guess this is ok. Could possibly add it to the existing terrible >>>>>>>> "enable this pass" interface on liblto as well. >>>>>>>> >>>>>>>> The linker doesn't know whether clang was built with asserts, though. >>>>>>>> >>>>>>>> We could just make it implicit: move the decision to libLTO itself. Given >>>>>>>> that clang and libLTO.dylib are different executables anyway -- and you >>>>>>>> might be interposing an asserts libLTO.dylib to use with an installed clang >>>>>>>> -- maybe it's even better? >>>>>>>> >>>>>>>> >>>>>>> *nod* We could do that. Seems better than the alternative. >>>>>> >>>>>> +1 >>>>> >>>>> Finally got back to this. Done in r247729. >>>>> >>>>> I didn't modify gold-plugin.cpp, as I don't have a good way to test it, >>>>> but someone else should be able to do it pretty easily. >>>> >>>> I can do this for gold (presumably also controlled via an option, but >>>> set default based on NDEBUG). >>>> >>>> Couple questions: >>>> - For your patch the default is set based on NDEBUG for lto.cpp, but >>>> in llvm-lto it always defaults to false. Is that intentional? >>> >>> After writing a test for my gold-plugin changes, I think I know the >>> answer to the above question. It is presumably because you don't want >>> the behavior of new test ./test/LTO/X86/disable-verify.ll which uses >>> llvm-lto to change based on whether the compiler is built NDEBUG or >>> not, since you are also testing the default behavior without >>> -disable-verify in this test. >>> >>> Since gold-plugin isn't a testing tool, I think we do want the default >>> controlled by NDEBUG. So presumably my new gold-plugin-based test >>> cannot test the default behavior, just the behavior with the new >>> disable-verify plugin option. >> >> Note sure if it would help but you can add “REQUIRE: assert” in a test to only run it when NDEBUG is enabled. > > Wouldn't asserts be off generally in NDEBUG mode? And if so does the > "assert" check here always imply !NDEBUG? If so, that would do the > trick.Yes, I slipped, I meant “enabled” as when “assertions are enabled”, so NDEBUG is unset.> > Also, is there documentation on the REQUIRE values? I couldn't find > any under http://llvm.org/docs/TestingGuide.htmlI don’t know where is it documented (if any), but AFAIK most “features” are defined programmatically in test/lit.cfg: $ grep feature test/lit.cfg config.available_features.add('shell') config.available_features.add('can-execute') config.available_features.add('loadable_module') config.available_features.add("asan") config.available_features.add("msan") config.available_features.add("ubsan") config.available_features.add("not_ubsan") config.available_features.add("long_tests") config.available_features.add("object-emission") config.available_features.add("zlib") config.available_features.add("nozlib") config.available_features.add("default_triple") config.available_features.add("native") config.available_features.add('ld_emu_elf32ppc') config.available_features.add('ld_plugin') config.available_features.add('ld64_plugin') config.available_features.add('asserts') config.available_features.add('fma3') config.available_features.add('debug_frame') It seems that there are some features that are native to lit: “valgrind” and “vg_leak”. — Mehdi> > Thanks, > Teresa > >> >> There is no “REQUIRE: noassert” though. >> >> >> — >> Mehdi >> >> >> >>> >>> Teresa >>> >>>> - You mentioned that the verifier was currently being run 3 times: (1) >>>> after parsing, (2) at the beginning of the optimization pipeline, and >>>> (3) at the end of it. It looks to me like (1) is done via the >>>> createVerifierPass() added in >>>> LTOCodeGenerator::applyScopeRestrictions(). However, gold does not use >>>> LTOCodeGenerator, and I don't see it explicitly adding an initial >>>> createVerifierPass. So it looks like for gold it is only being called >>>> twice (beginning of optimization pipeline and at the end). So I think >>>> for gold I need to leave VerifyInput on the pass manager builder set >>>> to true unconditionally in order to get an initial round of input >>>> verification. Does that sound right? >>>> >>>> Teresa >>>> >>>>> _______________________________________________ >>>>> LLVM Developers mailing list >>>>> llvm-dev at lists.llvm.org >>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>>> >>>> >>>> >>>> -- >>>> Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413 >>> >>> >>> >>> -- >>> Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413 >>> _______________________________________________ >>> LLVM Developers mailing list >>> llvm-dev at lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> > > > > -- > Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413
Mehdi Amini via llvm-dev
2015-Sep-16 17:51 UTC
[llvm-dev] RFC: LTO should use -disable-llvm-verifier
> On Sep 16, 2015, at 10:46 AM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote: > > >> On 2015-Sep-16, at 10:09, Mehdi Amini <mehdi.amini at apple.com> wrote: >> >>> >>> On Sep 16, 2015, at 9:45 AM, Teresa Johnson via llvm-dev <llvm-dev at lists.llvm.org> wrote: >>> >>> On Wed, Sep 16, 2015 at 7:47 AM, Teresa Johnson <tejohnson at google.com> wrote: >>>> On Tue, Sep 15, 2015 at 3:31 PM, Duncan P. N. Exon Smith via llvm-dev >>>> <llvm-dev at lists.llvm.org> wrote: >>>>> >>>>>> On 2015-Sep-02, at 19:31, Peter Collingbourne <peter at pcc.me.uk> wrote: >>>>>> >>>>>> On Thu, Sep 03, 2015 at 01:10:42AM +0000, Eric Christopher wrote: >>>>>>> On Tue, Sep 1, 2015 at 10:43 AM Duncan P. N. Exon Smith < >>>>>>> dexonsmith at apple.com> wrote: >>>>>>> >>>>>>>> >>>>>>>>> On 2015-Aug-31, at 18:09, Eric Christopher <echristo at gmail.com> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> On Mon, Aug 31, 2015 at 5:50 PM Duncan P. N. Exon Smith < >>>>>>>> dexonsmith at apple.com> wrote: >>>>>>>>> >>>>>>>>>> On 2015-Aug-31, at 12:21, Eric Christopher <echristo at gmail.com> wrote: >>>>>>>>>> Yep. This is where I was going :) >>>>>>>>> >>>>>>>>> Glad I found consensus, but I want to double-check that this makes >>>>>>>>> sense to add to the driver. I didn't quite think through the >>>>>>>>> implications myself. >>>>>>>>> >>>>>>>>> Since the driver doesn't know if there's any bitcode, or if LTO is >>>>>>>>> going to be invoked, it seems like I'll have to change the noasserts >>>>>>>>> driver to *always* pass the option to the linker just in case we are >>>>>>>>> doing LTO. Is this reasonable? >>>>>>>>> >>>>>>>>> Also, I realized that passing `-mllvm -disable-llvm-verifier` to ld64 >>>>>>>>> is redundant... so I'm thinking `-mllvm -disable-verify`. Make >>>>>>>>> sense? >>>>>>>>> >>>>>>>>> *sigh* Reasons to hate the driver interface again... >>>>>>>>> >>>>>>>>> I guess this is ok. Could possibly add it to the existing terrible >>>>>>>> "enable this pass" interface on liblto as well. >>>>>>>> >>>>>>>> The linker doesn't know whether clang was built with asserts, though. >>>>>>>> >>>>>>>> We could just make it implicit: move the decision to libLTO itself. Given >>>>>>>> that clang and libLTO.dylib are different executables anyway -- and you >>>>>>>> might be interposing an asserts libLTO.dylib to use with an installed clang >>>>>>>> -- maybe it's even better? >>>>>>>> >>>>>>>> >>>>>>> *nod* We could do that. Seems better than the alternative. >>>>>> >>>>>> +1 >>>>> >>>>> Finally got back to this. Done in r247729. >>>>> >>>>> I didn't modify gold-plugin.cpp, as I don't have a good way to test it, >>>>> but someone else should be able to do it pretty easily. >>>> >>>> I can do this for gold (presumably also controlled via an option, but >>>> set default based on NDEBUG). >>>> >>>> Couple questions: >>>> - For your patch the default is set based on NDEBUG for lto.cpp, but >>>> in llvm-lto it always defaults to false. Is that intentional? >>> >>> After writing a test for my gold-plugin changes, I think I know the >>> answer to the above question. It is presumably because you don't want >>> the behavior of new test ./test/LTO/X86/disable-verify.ll which uses >>> llvm-lto to change based on whether the compiler is built NDEBUG or >>> not, since you are also testing the default behavior without >>> -disable-verify in this test. >>> >>> Since gold-plugin isn't a testing tool, I think we do want the default >>> controlled by NDEBUG. So presumably my new gold-plugin-based test >>> cannot test the default behavior, just the behavior with the new >>> disable-verify plugin option. >> >> Note sure if it would help but you can add “REQUIRE: assert” in a test to only run it when NDEBUG is enabled. >> >> There is no “REQUIRE: noassert” though. > > It would be possible, but I like that llvm-lto (as a testing tool) behaves > consistently regardless of assertions.It was a suggestion to be able to test gold, not for llvm-lto. — Mehdi>> >> >> >> >>> >>> Teresa >>> >>>> - You mentioned that the verifier was currently being run 3 times: (1) >>>> after parsing, (2) at the beginning of the optimization pipeline, and >>>> (3) at the end of it. It looks to me like (1) is done via the >>>> createVerifierPass() added in >>>> LTOCodeGenerator::applyScopeRestrictions(). However, gold does not use >>>> LTOCodeGenerator, and I don't see it explicitly adding an initial >>>> createVerifierPass. So it looks like for gold it is only being called >>>> twice (beginning of optimization pipeline and at the end). So I think >>>> for gold I need to leave VerifyInput on the pass manager builder set >>>> to true unconditionally in order to get an initial round of input >>>> verification. Does that sound right? >>>> >>>> Teresa >>>> >>>>> _______________________________________________ >>>>> LLVM Developers mailing list >>>>> llvm-dev at lists.llvm.org >>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>>> >>>> >>>> >>>> -- >>>> Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413 >>> >>> >>> >>> -- >>> Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413 >>> _______________________________________________ >>> LLVM Developers mailing list >>> llvm-dev at lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >