Re: [Mesa-dev] [PATCH] opencl: autotools: Fix linking order for OpenCL target

[Emil Velikov]

Hi all,

Seems like I've got this stuck in the drafts queue. While the patch
has been merged, I'm hitting "Send" for posterity.

On 2 May 2018 at 21:38, Jan Vesely  wrote:

> @Emil, are you OK with this patch?
>
In a Tl;Dr; yes, patch is fine and is
Reviewed-by: Emil Velikov 

A bit more detail on the topic in general.

Autotools does not support positional arguments (like
(start|end)-group) as some of those are not portable. As in some of
the platforms it "supports" do not have an equivalents.
libtool creates a "whole-archive" libraries (the .la ones), which
effectively does the same thing.

The as-needed option is the one which [immediately] discards any
static or shared objects if they're not used by an object preceding
it. Although these days distributions flip the gcc/binutils configure
which that does effectively the same thing. IIRC Debian was the last
one an year or so ago.

HTH
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] opencl: autotools: Fix linking order for OpenCL target

[Kai Wasserbäch]

Hey Jan,
Jan Vesely wrote on 15.05.2018 04:50:
> I've pushed the patch.

thank you very much! I went ahead and closed the associated bug in the meantime.

> I've also kept the stable tag although the
> policy is (afaik) to only support llvm versions available at the point
> of mesa release. Don't be surprised to see some pushback from stable
> maintainers.

Ok, thanks for the heads-up. I won't fight too hard, since I'm only interested
in master. Though I do think it might be a good idea for at least 18.1. Anyway,
we'll see what the stable release managers think. ☺️

Cheers,
Kai



signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] opencl: autotools: Fix linking order for OpenCL target

[Jan Vesely]

Hi Kai,

I've pushed the patch. I've also kept the stable tag although the
policy is (afaik) to only support llvm versions available at the point
of mesa release. Don't be surprised to see some pushback from stable
maintainers.

thanks,
Jan

On Sun, 2018-05-13 at 23:19 +0200, Dieter Nützel wrote:
> Hello Jan and Kai,
> 
> would be nice if this lands (have T-b from me).
> 
> Dieter
> 
> Am 13.05.2018 19:10, schrieb Jan Vesely:
> > Hi,
> > 
> > sorry for the delay. I thought maybe Emil is on holiday. I plan to
> > push this on Monday evening (EDT) if there is no response by then.
> > 
> > Jan
> > 
> > On Sun, May 13, 2018 at 3:56 AM, Kai Wasserbäch
> >  wrote:
> > 
> > > Ping! Can somebody *please* commit this patch? It fixes an FTBFS,
> > > has two T-b
> > > and one A-b.
> > > 
> > > Kai Wasserbäch wrote on 07.05.2018 16:48:
> > > > Jan Vesely wrote on 02.05.2018 22:38:
> > > > > On Wed, 2018-05-02 at 18:38 +0200, Kai Wasserbäch wrote:
> > > > > > [...]
> > > > > 
> > > > > Thank for looking into this. We probably need CLANG_LIBS handling
> > > > > similar to LLVM_LIBS. I agree this is the best fix for now.
> > > > > 
> > > > > Acked-by: Jan Vesely 
> > > > > 
> > > > > libclang.so might be a solkution, but I'm not sure how it
> > > 
> > > interacts
> > > > > with older or static build clang. It's also weird that we are
> > > 
> > > linking
> > > > > to clang here instead of clover which actually uses clang
> > > 
> > > symbols.
> > > > > 
> > > > > @Emil, are you OK with this patch?
> > > > 
> > > > Gentle ping.
> > > > 
> > > > > > > > > > -lclangDriver \
> > > > > > > > > > -lclangSerialization \
> > > > > > > > > > -   -lclangCodeGen \
> > > > > > > > > 
> > > > > > > > > Is this change related?
> > > > > > > > 
> > > > > > > > Not really, just a minor clean-up while I was busy a few lines
> > > 
> > > above.
> > > > > > > > "clangCodeGen" is already named on the first Clang library
> > > 
> > > line.
> > > > > > > 
> > > > > > > ah, all right, maybe mention it in the commit message?
> > > > > > 
> > > > > > Do I need to resend the patch for that or can you just add a
> > > 
> > > line like "This
> > > > > > change also removes the duplicate clangCodeGen line (trivial
> > > 
> > > change)." before
> > > > > > pushing, considering, that there are two T-b tags to be added
> > > 
> > > anyway?
> > > > > 
> > > > > I'll add it on my side before pushing the patch.
> > > > 
> > > > Thanks a lot!
> > > > 
> > > > Cheers,
> > > > Kai
> > > > 
> > > > 
> > > > 
> > > > ___
> > > > mesa-dev mailing list
> > > > mesa-dev@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev [1]
> > > > 
> > > 
> > > --
> > > 
> > > Kai Wasserbäch (Kai Wasserbaech)
> > > 
> > > E-Mail: k...@dev.carbon-project.org
> > 
> > 
> > 
> > Links:
> > --
> > [1] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev

-- 
Jan Vesely 

signature.asc
Description: This is a digitally signed message part
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] opencl: autotools: Fix linking order for OpenCL target

[Dieter Nützel]

Hello Jan and Kai,

would be nice if this lands (have T-b from me).

Dieter

Am 13.05.2018 19:10, schrieb Jan Vesely:

Hi,

sorry for the delay. I thought maybe Emil is on holiday. I plan to
push this on Monday evening (EDT) if there is no response by then.

Jan

On Sun, May 13, 2018 at 3:56 AM, Kai Wasserbäch
 wrote:


Ping! Can somebody *please* commit this patch? It fixes an FTBFS,
has two T-b
and one A-b.

Kai Wasserbäch wrote on 07.05.2018 16:48:

Jan Vesely wrote on 02.05.2018 22:38:

On Wed, 2018-05-02 at 18:38 +0200, Kai Wasserbäch wrote:

[...]


Thank for looking into this. We probably need CLANG_LIBS handling
similar to LLVM_LIBS. I agree this is the best fix for now.

Acked-by: Jan Vesely 

libclang.so might be a solkution, but I'm not sure how it

interacts

with older or static build clang. It's also weird that we are

linking

to clang here instead of clover which actually uses clang

symbols.


@Emil, are you OK with this patch?


Gentle ping.


-lclangDriver \
-lclangSerialization \
-   -lclangCodeGen \


Is this change related?


Not really, just a minor clean-up while I was busy a few lines

above.

"clangCodeGen" is already named on the first Clang library

line.


ah, all right, maybe mention it in the commit message?


Do I need to resend the patch for that or can you just add a

line like "This

change also removes the duplicate clangCodeGen line (trivial

change)." before

pushing, considering, that there are two T-b tags to be added

anyway?


I'll add it on my side before pushing the patch.


Thanks a lot!

Cheers,
Kai






___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev [1]



--

Kai Wasserbäch (Kai Wasserbaech)

E-Mail: k...@dev.carbon-project.org




Links:
--
[1] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] opencl: autotools: Fix linking order for OpenCL target

[Jan Vesely]

Hi,

sorry for the delay. I thought maybe Emil is on holiday. I plan to push
this on Monday evening (EDT) if there is no response by then.

Jan

On Sun, May 13, 2018 at 3:56 AM, Kai Wasserbäch 
wrote:

> Ping! Can somebody *please* commit this patch? It fixes an FTBFS, has two
> T-b
> and one A-b.
>
>
> Kai Wasserbäch wrote on 07.05.2018 16:48:
> > Jan Vesely wrote on 02.05.2018 22:38:
> >> On Wed, 2018-05-02 at 18:38 +0200, Kai Wasserbäch wrote:
> >>> [...]
> >>
> >> Thank for looking into this. We probably need CLANG_LIBS handling
> >> similar to LLVM_LIBS. I agree this is the best fix for now.
> >>
> >> Acked-by: Jan Vesely 
> >>
> >> libclang.so might be a solkution, but I'm not sure how it interacts
> >> with older or static build clang. It's also weird that we are linking
> >> to clang here instead of clover which actually uses clang symbols.
> >>
> >> @Emil, are you OK with this patch?
> >
> > Gentle ping.
> >
> >>> -lclangDriver \
> >>> -lclangSerialization \
> >>> -   -lclangCodeGen \
> >>
> >> Is this change related?
> >
> > Not really, just a minor clean-up while I was busy a few lines above.
> > "clangCodeGen" is already named on the first Clang library line.
> 
>  ah, all right, maybe mention it in the commit message?
> >>>
> >>> Do I need to resend the patch for that or can you just add a line like
> "This
> >>> change also removes the duplicate clangCodeGen line (trivial change)."
> before
> >>> pushing, considering, that there are two T-b tags to be added anyway?
> >>
> >> I'll add it on my side before pushing the patch.
> >
> > Thanks a lot!
> >
> > Cheers,
> > Kai
> >
> >
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
>
> --
>
> Kai Wasserbäch (Kai Wasserbaech)
>
> E-Mail: k...@dev.carbon-project.org
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] opencl: autotools: Fix linking order for OpenCL target

[Kai Wasserbäch]

Ping! Can somebody *please* commit this patch? It fixes an FTBFS, has two T-b
and one A-b.


Kai Wasserbäch wrote on 07.05.2018 16:48:
> Jan Vesely wrote on 02.05.2018 22:38:
>> On Wed, 2018-05-02 at 18:38 +0200, Kai Wasserbäch wrote:
>>> [...]
>>
>> Thank for looking into this. We probably need CLANG_LIBS handling
>> similar to LLVM_LIBS. I agree this is the best fix for now.
>>
>> Acked-by: Jan Vesely 
>>
>> libclang.so might be a solkution, but I'm not sure how it interacts
>> with older or static build clang. It's also weird that we are linking
>> to clang here instead of clover which actually uses clang symbols.
>>
>> @Emil, are you OK with this patch?
> 
> Gentle ping.
> 
>>> -lclangDriver \
>>> -lclangSerialization \
>>> -   -lclangCodeGen \
>>
>> Is this change related?
>
> Not really, just a minor clean-up while I was busy a few lines above.
> "clangCodeGen" is already named on the first Clang library line.

 ah, all right, maybe mention it in the commit message?
>>>
>>> Do I need to resend the patch for that or can you just add a line like "This
>>> change also removes the duplicate clangCodeGen line (trivial change)." 
>>> before
>>> pushing, considering, that there are two T-b tags to be added anyway?
>>
>> I'll add it on my side before pushing the patch.
> 
> Thanks a lot!
> 
> Cheers,
> Kai
> 
> 
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 

-- 

Kai Wasserbäch (Kai Wasserbaech)

E-Mail: k...@dev.carbon-project.org



signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] opencl: autotools: Fix linking order for OpenCL target

[Kai Wasserbäch]

Jan Vesely wrote on 02.05.2018 22:38:
> On Wed, 2018-05-02 at 18:38 +0200, Kai Wasserbäch wrote:
>> [...]
> 
> Thank for looking into this. We probably need CLANG_LIBS handling
> similar to LLVM_LIBS. I agree this is the best fix for now.
> 
> Acked-by: Jan Vesely 
> 
> libclang.so might be a solkution, but I'm not sure how it interacts
> with older or static build clang. It's also weird that we are linking
> to clang here instead of clover which actually uses clang symbols.
> 
> @Emil, are you OK with this patch?

Gentle ping.

>>  -lclangDriver \
>>  -lclangSerialization \
>> --lclangCodeGen \
>
> Is this change related?

 Not really, just a minor clean-up while I was busy a few lines above.
 "clangCodeGen" is already named on the first Clang library line.
>>>
>>> ah, all right, maybe mention it in the commit message?
>>
>> Do I need to resend the patch for that or can you just add a line like "This
>> change also removes the duplicate clangCodeGen line (trivial change)." before
>> pushing, considering, that there are two T-b tags to be added anyway?
> 
> I'll add it on my side before pushing the patch.

Thanks a lot!

Cheers,
Kai



signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] opencl: autotools: Fix linking order for OpenCL target

[Jan Vesely]

On Wed, 2018-05-02 at 18:38 +0200, Kai Wasserbäch wrote:
> Hey Jan,
> Jan Vesely wrote on 01.05.2018 23:59:
> > On Tue, 2018-05-01 at 18:23 +0200, Kai Wasserbäch wrote:
> > > Jan Vesely wrote on 01.05.2018 17:19:
> > > > On Tue, 2018-05-01 at 14:14 +0200, Kai Wasserbäch wrote:
> > > > > [...]
> > > > > 
> > > > >  src/gallium/targets/opencl/Makefile.am | 3 +--
> > > > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/src/gallium/targets/opencl/Makefile.am 
> > > > > b/src/gallium/targets/opencl/Makefile.am
> > > > > index de68a93ad5..f0e1de7797 100644
> > > > > --- a/src/gallium/targets/opencl/Makefile.am
> > > > > +++ b/src/gallium/targets/opencl/Makefile.am
> > > > > @@ -23,11 +23,10 @@ lib@OPENCL_LIBNAME@_la_LIBADD = \
> > > > >   $(LIBELF_LIBS) \
> > > > >   $(DLOPEN_LIBS) \
> > > > >   -lclangCodeGen \
> > > > > - -lclangFrontendTool \
> > > > >   -lclangFrontend \
> > > > > + -lclangFrontendTool \
> > > > 
> > > > This is strange. Why does reordering help here? Do we use -Wl,--as-
> > > > needed anywhere?
> > > 
> > > No, not that I can see.
> > > 
> > > > Should we use -Wl,--start-group/-Wl,--end-group for all clang libraries
> > > > instead?
> > > 
> > > Maybe? This was the simplest fix I could come up with, but if there's a
> > > preference for a link group, I can give that a try as well.
> > 
> > So the fix is to change ordering?
> 
> yes.
> 
> > Does using groups fix the issue as well? I think that would be
> > preferable, but I use split .so files, so I don't hit this issue.
> 
> I tried convincing autotools to work with those flags but failed. The only
> option I see to solve this, is very messy IMHO (and would still need the
> ordering fix): putting -Wl,--{start,end}-group directly into the right places 
> in
> lib@OPENCL_LIBNAME@_la_LIBADD is forbidden by automake ("error: linker flags
> such as '-Wl,--start-group' belong in 'lib@OPENCL_LIBNAME@_la_LDFLAGS'") and
> adding them to lib@OPENCL_LIBNAME@_la_LDFLAGS like automake is suggesting 
> won't
> work for obvious reasons. The only solution I can see is to work with
> substitution because automake seems to "not see" the flags then. I could do an
> unconditional replacement, but there are probably linkers with no support for
> these flags, which would mean I'd have to do the ordering fix in any case and
> then conditionally set "-Wl,--{start,end}-group" just for the GNU toolchain 
> with
> no immediate benefit beyond future-proofing this section.
> But maybe people who are deeper into the whole autotools stuff (Emil?
> Francisco?) can point me to a solution? Otherwise I'd like to return to my
> original patch which fixes the FTBFS and works for now. Or maybe the library
> could be linked against libclang.so (at least when --enable-llvm-shared-libs 
> is set?

Thank for looking into this. We probably need CLANG_LIBS handling
similar to LLVM_LIBS. I agree this is the best fix for now.

Acked-by: Jan Vesely 

libclang.so might be a solkution, but I'm not sure how it interacts
with older or static build clang. It's also weird that we are linking
to clang here instead of clover which actually uses clang symbols.

@Emil, are you OK with this patch?

> 
> > > > >   -lclangDriver \
> > > > >   -lclangSerialization \
> > > > > - -lclangCodeGen \
> > > > 
> > > > Is this change related?
> > > 
> > > Not really, just a minor clean-up while I was busy a few lines above.
> > > "clangCodeGen" is already named on the first Clang library line.
> > 
> > ah, all right, maybe mention it in the commit message?
> 
> Do I need to resend the patch for that or can you just add a line like "This
> change also removes the duplicate clangCodeGen line (trivial change)." before
> pushing, considering, that there are two T-b tags to be added anyway?

I'll add it on my side before pushing the patch.

thanks,
Jan

> 
> Cheers,
> Kai
> 


signature.asc
Description: This is a digitally signed message part
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] opencl: autotools: Fix linking order for OpenCL target

[Kai Wasserbäch]

Hey Jan,
Jan Vesely wrote on 01.05.2018 23:59:
> On Tue, 2018-05-01 at 18:23 +0200, Kai Wasserbäch wrote:
>> Jan Vesely wrote on 01.05.2018 17:19:
>>> On Tue, 2018-05-01 at 14:14 +0200, Kai Wasserbäch wrote:
 [...]

  src/gallium/targets/opencl/Makefile.am | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

 diff --git a/src/gallium/targets/opencl/Makefile.am 
 b/src/gallium/targets/opencl/Makefile.am
 index de68a93ad5..f0e1de7797 100644
 --- a/src/gallium/targets/opencl/Makefile.am
 +++ b/src/gallium/targets/opencl/Makefile.am
 @@ -23,11 +23,10 @@ lib@OPENCL_LIBNAME@_la_LIBADD = \
$(LIBELF_LIBS) \
$(DLOPEN_LIBS) \
-lclangCodeGen \
 -  -lclangFrontendTool \
-lclangFrontend \
 +  -lclangFrontendTool \
>>>
>>> This is strange. Why does reordering help here? Do we use -Wl,--as-
>>> needed anywhere?
>>
>> No, not that I can see.
>>
>>> Should we use -Wl,--start-group/-Wl,--end-group for all clang libraries
>>> instead?
>>
>> Maybe? This was the simplest fix I could come up with, but if there's a
>> preference for a link group, I can give that a try as well.
> 
> So the fix is to change ordering?

yes.

> Does using groups fix the issue as well? I think that would be
> preferable, but I use split .so files, so I don't hit this issue.

I tried convincing autotools to work with those flags but failed. The only
option I see to solve this, is very messy IMHO (and would still need the
ordering fix): putting -Wl,--{start,end}-group directly into the right places in
lib@OPENCL_LIBNAME@_la_LIBADD is forbidden by automake ("error: linker flags
such as '-Wl,--start-group' belong in 'lib@OPENCL_LIBNAME@_la_LDFLAGS'") and
adding them to lib@OPENCL_LIBNAME@_la_LDFLAGS like automake is suggesting won't
work for obvious reasons. The only solution I can see is to work with
substitution because automake seems to "not see" the flags then. I could do an
unconditional replacement, but there are probably linkers with no support for
these flags, which would mean I'd have to do the ordering fix in any case and
then conditionally set "-Wl,--{start,end}-group" just for the GNU toolchain with
no immediate benefit beyond future-proofing this section.
But maybe people who are deeper into the whole autotools stuff (Emil?
Francisco?) can point me to a solution? Otherwise I'd like to return to my
original patch which fixes the FTBFS and works for now. Or maybe the library
could be linked against libclang.so (at least when --enable-llvm-shared-libs is 
set?

-lclangDriver \
-lclangSerialization \
 -  -lclangCodeGen \
>>>
>>> Is this change related?
>>
>> Not really, just a minor clean-up while I was busy a few lines above.
>> "clangCodeGen" is already named on the first Clang library line.
> 
> ah, all right, maybe mention it in the commit message?

Do I need to resend the patch for that or can you just add a line like "This
change also removes the duplicate clangCodeGen line (trivial change)." before
pushing, considering, that there are two T-b tags to be added anyway?

Cheers,
Kai



signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] opencl: autotools: Fix linking order for OpenCL target

[Dieter Nützel]

Tested-by: Dieter Nützel 

Dieter

Am 01.05.2018 14:14, schrieb Kai Wasserbäch:

Otherwise the build fails with an undefined reference to
clang::FrontendTimesIsEnabled.

Bugzilla: https://bugs.freedesktop.org/106209
Cc: mesa-sta...@lists.freedesktop.org
Cc: Jan Vesely 
Signed-off-by: Kai Wasserbäch 
---

Hey,
this patch fixes a FTBFS for me with recent LLVM/Clang 7 revisions from
upstream's SVN (I use the packages from apt.llvm.org).

If you accept it, please commit it for me, I do not have commit access.

The CC to stable can be dropped, if stable branches are not to be
expected to be buildable with LLVM/Clang from SVN.

Thank you in advance for considering this patch.

Cheers,
Kai


 src/gallium/targets/opencl/Makefile.am | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/gallium/targets/opencl/Makefile.am
b/src/gallium/targets/opencl/Makefile.am
index de68a93ad5..f0e1de7797 100644
--- a/src/gallium/targets/opencl/Makefile.am
+++ b/src/gallium/targets/opencl/Makefile.am
@@ -23,11 +23,10 @@ lib@OPENCL_LIBNAME@_la_LIBADD = \
$(LIBELF_LIBS) \
$(DLOPEN_LIBS) \
-lclangCodeGen \
-   -lclangFrontendTool \
-lclangFrontend \
+   -lclangFrontendTool \
-lclangDriver \
-lclangSerialization \
-   -lclangCodeGen \
-lclangParse \
-lclangSema \
-lclangAnalysis \

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] opencl: autotools: Fix linking order for OpenCL target

[Jan Vesely]

On Tue, 2018-05-01 at 18:23 +0200, Kai Wasserbäch wrote:
> Hey Jan,
> Jan Vesely wrote on 01.05.2018 17:19:
> > On Tue, 2018-05-01 at 14:14 +0200, Kai Wasserbäch wrote:
> > > Otherwise the build fails with an undefined reference to
> > > clang::FrontendTimesIsEnabled.
> > > 
> > > Bugzilla: https://bugs.freedesktop.org/106209
> > > Cc: mesa-sta...@lists.freedesktop.org
> > > Cc: Jan Vesely 
> > > Signed-off-by: Kai Wasserbäch 
> > > ---
> > > 
> > > Hey,
> > > this patch fixes a FTBFS for me with recent LLVM/Clang 7 revisions from
> > > upstream's SVN (I use the packages from apt.llvm.org).
> > > 
> > > If you accept it, please commit it for me, I do not have commit access.
> > > 
> > > The CC to stable can be dropped, if stable branches are not to be
> > > expected to be buildable with LLVM/Clang from SVN.
> > 
> > thanks for looking into this. TBH I don't understand how this patch
> > works, it's dropping and reordering linked libraries.
> > I've added Emil and Francisco to cc.
> 
> the removal is just the removal of a duplicate line (the first Clang library
> named is already "clangCodeGen"). And AFAICT there's no "-Wl,--as-needed" in
> use. At least I don't find anything in the build logs.
> 
> > >  src/gallium/targets/opencl/Makefile.am | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > 
> > > diff --git a/src/gallium/targets/opencl/Makefile.am 
> > > b/src/gallium/targets/opencl/Makefile.am
> > > index de68a93ad5..f0e1de7797 100644
> > > --- a/src/gallium/targets/opencl/Makefile.am
> > > +++ b/src/gallium/targets/opencl/Makefile.am
> > > @@ -23,11 +23,10 @@ lib@OPENCL_LIBNAME@_la_LIBADD = \
> > >   $(LIBELF_LIBS) \
> > >   $(DLOPEN_LIBS) \
> > >   -lclangCodeGen \
> > > - -lclangFrontendTool \
> > >   -lclangFrontend \
> > > + -lclangFrontendTool \
> > 
> > This is strange. Why does reordering help here? Do we use -Wl,--as-
> > needed anywhere?
> 
> No, not that I can see.
> 
> > Should we use -Wl,--start-group/-Wl,--end-group for all clang libraries
> > instead?
> 
> Maybe? This was the simplest fix I could come up with, but if there's a
> preference for a link group, I can give that a try as well.

So the fix is to change ordering?
Does using groups fix the issue as well? I think that would be
preferable, but I use split .so files, so I don't hit this issue.

> 
> > >   -lclangDriver \
> > >   -lclangSerialization \
> > > - -lclangCodeGen \
> > 
> > Is this change related?
> 
> Not really, just a minor clean-up while I was busy a few lines above.
> "clangCodeGen" is already named on the first Clang library line.

ah, all right, maybe mention it in the commit message?

Jan
> 
> Cheers,
> Kai
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


signature.asc
Description: This is a digitally signed message part
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] opencl: autotools: Fix linking order for OpenCL target

[Kai Wasserbäch]

Hey Jan,
Jan Vesely wrote on 01.05.2018 17:19:
> On Tue, 2018-05-01 at 14:14 +0200, Kai Wasserbäch wrote:
>> Otherwise the build fails with an undefined reference to
>> clang::FrontendTimesIsEnabled.
>>
>> Bugzilla: https://bugs.freedesktop.org/106209
>> Cc: mesa-sta...@lists.freedesktop.org
>> Cc: Jan Vesely 
>> Signed-off-by: Kai Wasserbäch 
>> ---
>>
>> Hey,
>> this patch fixes a FTBFS for me with recent LLVM/Clang 7 revisions from
>> upstream's SVN (I use the packages from apt.llvm.org).
>>
>> If you accept it, please commit it for me, I do not have commit access.
>>
>> The CC to stable can be dropped, if stable branches are not to be
>> expected to be buildable with LLVM/Clang from SVN.
> 
> thanks for looking into this. TBH I don't understand how this patch
> works, it's dropping and reordering linked libraries.
> I've added Emil and Francisco to cc.

the removal is just the removal of a duplicate line (the first Clang library
named is already "clangCodeGen"). And AFAICT there's no "-Wl,--as-needed" in
use. At least I don't find anything in the build logs.

>>  src/gallium/targets/opencl/Makefile.am | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/src/gallium/targets/opencl/Makefile.am 
>> b/src/gallium/targets/opencl/Makefile.am
>> index de68a93ad5..f0e1de7797 100644
>> --- a/src/gallium/targets/opencl/Makefile.am
>> +++ b/src/gallium/targets/opencl/Makefile.am
>> @@ -23,11 +23,10 @@ lib@OPENCL_LIBNAME@_la_LIBADD = \
>>  $(LIBELF_LIBS) \
>>  $(DLOPEN_LIBS) \
>>  -lclangCodeGen \
>> --lclangFrontendTool \
>>  -lclangFrontend \
>> +-lclangFrontendTool \
> 
> This is strange. Why does reordering help here? Do we use -Wl,--as-
> needed anywhere?

No, not that I can see.

> Should we use -Wl,--start-group/-Wl,--end-group for all clang libraries
> instead?

Maybe? This was the simplest fix I could come up with, but if there's a
preference for a link group, I can give that a try as well.

>>  -lclangDriver \
>>  -lclangSerialization \
>> --lclangCodeGen \
> 
> Is this change related?

Not really, just a minor clean-up while I was busy a few lines above.
"clangCodeGen" is already named on the first Clang library line.

Cheers,
Kai



signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] opencl: autotools: Fix linking order for OpenCL target

[Jan Vesely]

On Tue, 2018-05-01 at 14:14 +0200, Kai Wasserbäch wrote:
> Otherwise the build fails with an undefined reference to
> clang::FrontendTimesIsEnabled.
> 
> Bugzilla: https://bugs.freedesktop.org/106209
> Cc: mesa-sta...@lists.freedesktop.org
> Cc: Jan Vesely 
> Signed-off-by: Kai Wasserbäch 
> ---
> 
> Hey,
> this patch fixes a FTBFS for me with recent LLVM/Clang 7 revisions from
> upstream's SVN (I use the packages from apt.llvm.org).
> 
> If you accept it, please commit it for me, I do not have commit access.
> 
> The CC to stable can be dropped, if stable branches are not to be
> expected to be buildable with LLVM/Clang from SVN.
> 
> Thank you in advance for considering this patch.

Hi,

thanks for looking into this. TBH I don't understand how this patch
works, it's dropping and reordering linked libraries.
I've added Emil and Francisco to cc.

> 
> Cheers,
> Kai
> 
> 
>  src/gallium/targets/opencl/Makefile.am | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/src/gallium/targets/opencl/Makefile.am 
> b/src/gallium/targets/opencl/Makefile.am
> index de68a93ad5..f0e1de7797 100644
> --- a/src/gallium/targets/opencl/Makefile.am
> +++ b/src/gallium/targets/opencl/Makefile.am
> @@ -23,11 +23,10 @@ lib@OPENCL_LIBNAME@_la_LIBADD = \
>   $(LIBELF_LIBS) \
>   $(DLOPEN_LIBS) \
>   -lclangCodeGen \
> - -lclangFrontendTool \
>   -lclangFrontend \
> + -lclangFrontendTool \

This is strange. Why does reordering help here? Do we use -Wl,--as-
needed anywhere?
Should we use -Wl,--start-group/-Wl,--end-group for all clang libraries
instead?

>   -lclangDriver \
>   -lclangSerialization \
> - -lclangCodeGen \

Is this change related?

>   -lclangParse \
>   -lclangSema \
>   -lclangAnalysis \

thanks,
Jan


signature.asc
Description: This is a digitally signed message part
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] opencl: autotools: Fix linking order for OpenCL target

[Kai Wasserbäch]

Otherwise the build fails with an undefined reference to
clang::FrontendTimesIsEnabled.

Bugzilla: https://bugs.freedesktop.org/106209
Cc: mesa-sta...@lists.freedesktop.org
Cc: Jan Vesely 
Signed-off-by: Kai Wasserbäch 
---

Hey,
this patch fixes a FTBFS for me with recent LLVM/Clang 7 revisions from
upstream's SVN (I use the packages from apt.llvm.org).

If you accept it, please commit it for me, I do not have commit access.

The CC to stable can be dropped, if stable branches are not to be
expected to be buildable with LLVM/Clang from SVN.

Thank you in advance for considering this patch.

Cheers,
Kai


 src/gallium/targets/opencl/Makefile.am | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/gallium/targets/opencl/Makefile.am 
b/src/gallium/targets/opencl/Makefile.am
index de68a93ad5..f0e1de7797 100644
--- a/src/gallium/targets/opencl/Makefile.am
+++ b/src/gallium/targets/opencl/Makefile.am
@@ -23,11 +23,10 @@ lib@OPENCL_LIBNAME@_la_LIBADD = \
$(LIBELF_LIBS) \
$(DLOPEN_LIBS) \
-lclangCodeGen \
-   -lclangFrontendTool \
-lclangFrontend \
+   -lclangFrontendTool \
-lclangDriver \
-lclangSerialization \
-   -lclangCodeGen \
-lclangParse \
-lclangSema \
-lclangAnalysis \
-- 
2.17.0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev