Hi Emil,
comments below.
Am Montag, 10. Oktober 2016, 13:36:15 CEST schrieben Sie:
> Hi Tobias,
>
> On 8 October 2016 at 16:55, Tobias Droste <tdro...@gmx.de> wrote:
> > After this change each driver can request LLVM with a specific version
> > and specific targets/components anywhere.
> >
> > For gallium "--enable-gallium-llvm" is only needed if at least one
> > driver calls "gallium_require_llvm()".
> > If the flag is set to auto it will default to no now if no driver with
> > "gallium_require_llvm()" is included.
> >
> > If gallium swrast driver is selected and LLVM is installed and
> > "--enable-gallium-llvm" is yes or auto, it defaults to llvmpipe,
> > otherwise to softpipe.
> >
> > Signed-off-by: Tobias Droste <tdro...@gmx.de>
> > ---
> >
> > configure.ac | 451
> > +++++++++++++++++++++++++++++++++-------------------------- 1 file
> > changed, 249 insertions(+), 202 deletions(-)
>
> Huge thanks for starting to unwravel this.
>
> Since the configure code is quite picky, please split out the code
> movement from the functionality changes.
This is not easy as moving the code breaks it. If it's just for review and
doesn't need to work then I could do that.
>
> > +llvm_add_component() {
> > + new_llvm_component=$1
> > + fail_if_not_present=$2
> > + driver_name=$3
> > +
> > + if test -z "$LLVM_COMPONENTS"; then
> > + # Make the string non zero length
> > + LLVM_COMPONENTS=" "
> > +
>
> I'm not sure that this will work. For example
> $unset FOO && export FOO=" " && test -z $FOO && echo A
> A
It works that way.
But you're right, it's probably better your way (below).
>
> > + # Required default components
> > + llvm_add_component "engine" "yes" $driver_name
> > + llvm_add_component "bitwriter" "yes" $driver_name
> > + llvm_add_component "mcjit" "yes" $driver_name
> > + llvm_add_component "mcdisassembler" "yes" $driver_name
> > +
>
> At the same time I'm not sure that calling a function recursively will
> end up well, in the long run.
>
> That said using add_compulsory (mandatory/other) and add_optional
> helpers would be better than the current "yes" "no", wouldn't it ?
I'm going with "add_*" and "add_*_optional" and remove the recursion.
>
> Thanks
> Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
