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

[Duncan P. N. Exon Smith via llvm-dev]

> On 2015-Sep-16, at 07:47, 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?

Yes.  llvm-lto is a testing tool, so I think we generally want to run
the verifier there.

> - 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?

Sounds right to me.  Another option would be to verify each module
before linking it into the merged module (add a new verifier call)
and set VerifyInput to false.

> 
> 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