llvm dev - Sep 2015 - RFC: LTO should use -disable-llvm-verifier

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
> > > I don't suppose ld64 could move to a model like we're
talking about with
> > lld that pcc is working on?
> >
> > What specifically?
> 
> 
> Ah, using the C++ interface to handle everything and not using libLTO at
> all.
> 
> He can speak more to this though.
The C++ interface is much more convenient for a C++ program to use, but
clients need to revlock themselves to LLVM in order to use it.

In theory the same does not apply to libLTO, but we do end up changing it
occasionally to add new APIs which ld64 promptly starts using, so it isn't
clear how much ld64 gains by relying on libLTO, or whether the burden on
in-tree clients is worth it (there are certainly a number of internal APIs
that are more clumsy as a result of needing to support libLTO's API; see
e.g. llvm::splitCodeGen's return value).

Thanks,
-- 
Peter

Hi,
> On Sep 2, 2015, at 7:31 PM, Peter Collingbourne via llvm-dev <llvm-dev
at lists.llvm.org> 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
> 
>>>> I don't suppose ld64 could move to a model like we're
talking about with
>>> lld that pcc is working on?
>>> 
>>> What specifically?
>> 
>> 
>> Ah, using the C++ interface to handle everything and not using libLTO
at
>> all.
>> 
>> He can speak more to this though.
> 
> The C++ interface is much more convenient for a C++ program to use, but
> clients need to revlock themselves to LLVM in order to use it.
> 
> In theory the same does not apply to libLTO, but we do end up changing it
> occasionally to add new APIs which ld64 promptly starts using, so it
isn't
> clear how much ld64 gains by relying on libLTO,
Backward/Forward compatibility? 
Drop any version of clang/libLTO and still be able to use the system provided
linker on any version of OS X?
Sounds like a valuable feature to me, which is what I believe was (is?) sought
by the C API in general (but that’s another story).

— 
Mehdi

> or whether the burden on
> in-tree clients is worth it (there are certainly a number of internal APIs
> that are more clumsy as a result of needing to support libLTO's API;
see
> e.g. llvm::splitCodeGen's return value).
> 
> Thanks,
> -- 
> Peter
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

On Thu, Sep 3, 2015 at 11:45 PM Mehdi Amini <mehdi.amini at apple.com>
wrote:
> Hi,
>
> > On Sep 2, 2015, at 7:31 PM, Peter Collingbourne via llvm-dev <
> llvm-dev at lists.llvm.org> 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
> >
> >>>> I don't suppose ld64 could move to a model like
we're talking about
> with
> >>> lld that pcc is working on?
> >>>
> >>> What specifically?
> >>
> >>
> >> Ah, using the C++ interface to handle everything and not using
libLTO at
> >> all.
> >>
> >> He can speak more to this though.
> >
> > The C++ interface is much more convenient for a C++ program to use,
but
> > clients need to revlock themselves to LLVM in order to use it.
> >
> > In theory the same does not apply to libLTO, but we do end up changing
it
> > occasionally to add new APIs which ld64 promptly starts using, so it
> isn't
> > clear how much ld64 gains by relying on libLTO,
>
> Backward/Forward compatibility?
> Drop any version of clang/libLTO and still be able to use the system
> provided linker on any version of OS X?
> Sounds like a valuable feature to me, which is what I believe was (is?)
> sought by the C API in general (but that’s another story).
>
>
Static linking against the parts of llvm you care about? There's nothing in
the ld64 build system that means it can't do this :)

And you already don't have forward compatibility because of what Peter said
in the first place. :)

Anyhow, I don't actually expect this to change. Also the C API has to
change, but yes that's a different thread.

-eric

> —
> Mehdi
>
>
> > or whether the burden on
> > in-tree clients is worth it (there are certainly a number of internal
> APIs
> > that are more clumsy as a result of needing to support libLTO's
API; see
> > e.g. llvm::splitCodeGen's return value).
> >
> > Thanks,
> > --
> > Peter
> > _______________________________________________
> > LLVM Developers mailing list
> > llvm-dev at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150904/e277df4e/attachment.html>

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

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