[llvm-dev] RFC: LTO should use -disable-llvm-verifier

Wed Sep 16 11:10:06 PDT 2015

On Wed, Sep 16, 2015 at 10:50 AM, Mehdi Amini <mehdi.amini at apple.com> wrote:
>
>> 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.

Ok, great, that works then.

The gold-plugin change along with the new test case were committed as r247824.

Teresa

>
>
>>
>> Also, is there documentation on the REQUIRE values? I couldn't find
>> any under http://llvm.org/docs/TestingGuide.html
>
> I 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
>

-- 
Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413