Re: [Mesa-dev] [PATCH v3 02/25] configure.ac: Add helper function for targets/components
Am Mittwoch, 12. Oktober 2016, 20:21:29 CEST schrieb Emil Velikov: > >> > >> If things error out here, that means that one should finish the other > >> refactoring before using these helpers ? > >> If that's getting too hairy, just add keep the function a simple > >> wrapper around LLVM_COMPONENTS (no LLVM_CONFIG or AC_MSG_ERROR) and > >> add those when it's safe. > > > > Right now this is just a wrapper around LLVM_COMPONENTS. > > Atm, is more than just a dummy wrapper around LLVM_COMPONENTS. > > > If a component does not exist it will later not generate any LLVM_LIBS > > that's why it also doesn't need to be in LLVM_COMPONENTS. > > If it actually is missing, you will only notice during linking / building > > (depending on wether headers are missing or not). > > Looking at the current code - you only _need_ the --components | grep > foo check for the inteljitevents case. > Where/how do you see an issue happening ? No issue. I can add that function later in the series. Just wanted to say that the function without the error message is practiaclly just a wrapper as the end result is the same. The grep does no harm! But I reorder this a little bit so it is really just a wrapper in the first patches > > -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 02/25] configure.ac: Add helper function for targets/components
On 12 October 2016 at 19:44, Tobias Droste wrote:
> Am Mittwoch, 12. Oktober 2016, 19:38:56 CEST schrieb Emil Velikov:
>> On 12 October 2016 at 19:01, Tobias Droste wrote:
>> > Am Mittwoch, 12. Oktober 2016, 09:52:51 CEST schrieb Emil Velikov:
>> >> On 12 October 2016 at 00:02, Tobias Droste wrote:
>> >> > Add functions to add and check targets/components.
>> >> > Not used in this patch.
>> >> >
>> >> > The error message in llvm_add_component is disabled until it doesn't
>> >> > break
>> >> > the build anymore. This is the same functionality as before where the
>> >> > components were added without a check.
>> >> >
>> >> > Signed-off-by: Tobias Droste
>> >> > ---
>> >> >
>> >> > configure.ac | 35 +++
>> >> > 1 file changed, 35 insertions(+)
>> >> >
>> >> > diff --git a/configure.ac b/configure.ac
>> >> > index bdd46bc..69421ff 100644
>> >> > --- a/configure.ac
>> >> > +++ b/configure.ac
>> >> > @@ -2196,7 +2196,42 @@ llvm_check_version_for() {
>> >> >
>> >> > fi
>> >> >
>> >> > }
>> >> >
>> >> > +llvm_add_default_components() {
>> >> > +driver_name=$1
>> >> >
>> >> > +# Required default components
>> >> > +llvm_add_component "engine" $driver_name
>> >> > +llvm_add_component "bitwriter" $driver_name
>> >> > +llvm_add_component "mcjit" $driver_name
>> >> > +llvm_add_component "mcdisassembler" $driver_name
>> >> > +
>> >> > +# Optional default components
>> >> > +if $LLVM_CONFIG --components | grep -iqw inteljitevents ; then
>> >> > +LLVM_COMPONENTS="${LLVM_COMPONENTS} inteljitevents"
>> >> > +fi
>> >> > +}
>> >> > +
>> >> > +llvm_add_component() {
>> >>
>> >> Nit: move this function before its user (llvm_add_default_components)
>> >> Idea: call this ...components (plural) and feed the all components at
>> >> once.
>> >
>> > Ok.
>>
>> Just realised that you're adding these only to move them further up
>> with later patch. What's the obstacle for doing so in the first place
>> ?
>
> Nothing, but I was already on path 20 when I realised it and was to lazy to
> switch the ordering of the patches ;-)
>
>>
>> >> > +new_llvm_component=$1
>> >> > +driver_name=$2
>> >> > +
>> >> > +if $LLVM_CONFIG --components | grep -iqw $new_llvm_component ;
>> >> > then
>> >> > +LLVM_COMPONENTS="${LLVM_COMPONENTS} ${new_llvm_component}"
>> >> > +#else
>> >> > +#AC_MSG_ERROR([LLVM component '$new_llvm_component' not
>> >> > enabled
>> >> > in your LLVM build. Required by $driver_name.])
>> >>
>> >> This function adds required components, correct ? Then the above two
>> >> lines should not be commented out.
>> >
>> > I can't enable this until later in the series as mentioned in the commit
>> > message.
>> > This is how it worked before (there were no errors for the components).
>> >
>> > At this point llvm_add_default_components will be added even if we don't
>> > need LLVM. This way someone without LLVM installed will get error
>> > messages even though he might just want to build the intel or softpipe
>> > driver.
>> > It will be enabled in Patch 24.
>>
>> If things error out here, that means that one should finish the other
>> refactoring before using these helpers ?
>> If that's getting too hairy, just add keep the function a simple
>> wrapper around LLVM_COMPONENTS (no LLVM_CONFIG or AC_MSG_ERROR) and
>> add those when it's safe.
>
> Right now this is just a wrapper around LLVM_COMPONENTS.
Atm, is more than just a dummy wrapper around LLVM_COMPONENTS.
> If a component does not exist it will later not generate any LLVM_LIBS that's
> why it also doesn't need to be in LLVM_COMPONENTS.
> If it actually is missing, you will only notice during linking / building
> (depending on wether headers are missing or not).
>
Looking at the current code - you only _need_ the --components | grep
foo check for the inteljitevents case.
Where/how do you see an issue happening ?
-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 02/25] configure.ac: Add helper function for targets/components
Am Mittwoch, 12. Oktober 2016, 19:38:56 CEST schrieb Emil Velikov:
> On 12 October 2016 at 19:01, Tobias Droste wrote:
> > Am Mittwoch, 12. Oktober 2016, 09:52:51 CEST schrieb Emil Velikov:
> >> On 12 October 2016 at 00:02, Tobias Droste wrote:
> >> > Add functions to add and check targets/components.
> >> > Not used in this patch.
> >> >
> >> > The error message in llvm_add_component is disabled until it doesn't
> >> > break
> >> > the build anymore. This is the same functionality as before where the
> >> > components were added without a check.
> >> >
> >> > Signed-off-by: Tobias Droste
> >> > ---
> >> >
> >> > configure.ac | 35 +++
> >> > 1 file changed, 35 insertions(+)
> >> >
> >> > diff --git a/configure.ac b/configure.ac
> >> > index bdd46bc..69421ff 100644
> >> > --- a/configure.ac
> >> > +++ b/configure.ac
> >> > @@ -2196,7 +2196,42 @@ llvm_check_version_for() {
> >> >
> >> > fi
> >> >
> >> > }
> >> >
> >> > +llvm_add_default_components() {
> >> > +driver_name=$1
> >> >
> >> > +# Required default components
> >> > +llvm_add_component "engine" $driver_name
> >> > +llvm_add_component "bitwriter" $driver_name
> >> > +llvm_add_component "mcjit" $driver_name
> >> > +llvm_add_component "mcdisassembler" $driver_name
> >> > +
> >> > +# Optional default components
> >> > +if $LLVM_CONFIG --components | grep -iqw inteljitevents ; then
> >> > +LLVM_COMPONENTS="${LLVM_COMPONENTS} inteljitevents"
> >> > +fi
> >> > +}
> >> > +
> >> > +llvm_add_component() {
> >>
> >> Nit: move this function before its user (llvm_add_default_components)
> >> Idea: call this ...components (plural) and feed the all components at
> >> once.
> >
> > Ok.
>
> Just realised that you're adding these only to move them further up
> with later patch. What's the obstacle for doing so in the first place
> ?
Nothing, but I was already on path 20 when I realised it and was to lazy to
switch the ordering of the patches ;-)
>
> >> > +new_llvm_component=$1
> >> > +driver_name=$2
> >> > +
> >> > +if $LLVM_CONFIG --components | grep -iqw $new_llvm_component ;
> >> > then
> >> > +LLVM_COMPONENTS="${LLVM_COMPONENTS} ${new_llvm_component}"
> >> > +#else
> >> > +#AC_MSG_ERROR([LLVM component '$new_llvm_component' not
> >> > enabled
> >> > in your LLVM build. Required by $driver_name.])
> >>
> >> This function adds required components, correct ? Then the above two
> >> lines should not be commented out.
> >
> > I can't enable this until later in the series as mentioned in the commit
> > message.
> > This is how it worked before (there were no errors for the components).
> >
> > At this point llvm_add_default_components will be added even if we don't
> > need LLVM. This way someone without LLVM installed will get error
> > messages even though he might just want to build the intel or softpipe
> > driver.
> > It will be enabled in Patch 24.
>
> If things error out here, that means that one should finish the other
> refactoring before using these helpers ?
> If that's getting too hairy, just add keep the function a simple
> wrapper around LLVM_COMPONENTS (no LLVM_CONFIG or AC_MSG_ERROR) and
> add those when it's safe.
Right now this is just a wrapper around LLVM_COMPONENTS.
If a component does not exist it will later not generate any LLVM_LIBS that's
why it also doesn't need to be in LLVM_COMPONENTS.
If it actually is missing, you will only notice during linking / building
(depending on wether headers are missing or not).
>
> -Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 02/25] configure.ac: Add helper function for targets/components
On 12 October 2016 at 19:01, Tobias Droste wrote:
> Am Mittwoch, 12. Oktober 2016, 09:52:51 CEST schrieb Emil Velikov:
>> On 12 October 2016 at 00:02, Tobias Droste wrote:
>> > Add functions to add and check targets/components.
>> > Not used in this patch.
>> >
>> > The error message in llvm_add_component is disabled until it doesn't break
>> > the build anymore. This is the same functionality as before where the
>> > components were added without a check.
>> >
>> > Signed-off-by: Tobias Droste
>> > ---
>> >
>> > configure.ac | 35 +++
>> > 1 file changed, 35 insertions(+)
>> >
>> > diff --git a/configure.ac b/configure.ac
>> > index bdd46bc..69421ff 100644
>> > --- a/configure.ac
>> > +++ b/configure.ac
>> > @@ -2196,7 +2196,42 @@ llvm_check_version_for() {
>> >
>> > fi
>> >
>> > }
>> >
>> > +llvm_add_default_components() {
>> > +driver_name=$1
>> >
>> > +# Required default components
>> > +llvm_add_component "engine" $driver_name
>> > +llvm_add_component "bitwriter" $driver_name
>> > +llvm_add_component "mcjit" $driver_name
>> > +llvm_add_component "mcdisassembler" $driver_name
>> > +
>> > +# Optional default components
>> > +if $LLVM_CONFIG --components | grep -iqw inteljitevents ; then
>> > +LLVM_COMPONENTS="${LLVM_COMPONENTS} inteljitevents"
>> > +fi
>> > +}
>> > +
>> > +llvm_add_component() {
>>
>> Nit: move this function before its user (llvm_add_default_components)
>> Idea: call this ...components (plural) and feed the all components at once.
>
> Ok.
>
Just realised that you're adding these only to move them further up
with later patch. What's the obstacle for doing so in the first place
?
>>
>> > +new_llvm_component=$1
>> > +driver_name=$2
>> > +
>> > +if $LLVM_CONFIG --components | grep -iqw $new_llvm_component ; then
>> > +LLVM_COMPONENTS="${LLVM_COMPONENTS} ${new_llvm_component}"
>> > +#else
>> > +#AC_MSG_ERROR([LLVM component '$new_llvm_component' not enabled
>> > in your LLVM build. Required by $driver_name.])
>> This function adds required components, correct ? Then the above two
>> lines should not be commented out.
>
> I can't enable this until later in the series as mentioned in the commit
> message.
> This is how it worked before (there were no errors for the components).
>
> At this point llvm_add_default_components will be added even if we don't need
> LLVM. This way someone without LLVM installed will get error messages even
> though he might just want to build the intel or softpipe driver.
> It will be enabled in Patch 24.
>
If things error out here, that means that one should finish the other
refactoring before using these helpers ?
If that's getting too hairy, just add keep the function a simple
wrapper around LLVM_COMPONENTS (no LLVM_CONFIG or AC_MSG_ERROR) and
add those when it's safe.
-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 02/25] configure.ac: Add helper function for targets/components
Am Mittwoch, 12. Oktober 2016, 09:52:51 CEST schrieb Emil Velikov:
> On 12 October 2016 at 00:02, Tobias Droste wrote:
> > Add functions to add and check targets/components.
> > Not used in this patch.
> >
> > The error message in llvm_add_component is disabled until it doesn't break
> > the build anymore. This is the same functionality as before where the
> > components were added without a check.
> >
> > Signed-off-by: Tobias Droste
> > ---
> >
> > configure.ac | 35 +++
> > 1 file changed, 35 insertions(+)
> >
> > diff --git a/configure.ac b/configure.ac
> > index bdd46bc..69421ff 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -2196,7 +2196,42 @@ llvm_check_version_for() {
> >
> > fi
> >
> > }
> >
> > +llvm_add_default_components() {
> > +driver_name=$1
> >
> > +# Required default components
> > +llvm_add_component "engine" $driver_name
> > +llvm_add_component "bitwriter" $driver_name
> > +llvm_add_component "mcjit" $driver_name
> > +llvm_add_component "mcdisassembler" $driver_name
> > +
> > +# Optional default components
> > +if $LLVM_CONFIG --components | grep -iqw inteljitevents ; then
> > +LLVM_COMPONENTS="${LLVM_COMPONENTS} inteljitevents"
> > +fi
> > +}
> > +
> > +llvm_add_component() {
>
> Nit: move this function before its user (llvm_add_default_components)
> Idea: call this ...components (plural) and feed the all components at once.
Ok.
>
> > +new_llvm_component=$1
> > +driver_name=$2
> > +
> > +if $LLVM_CONFIG --components | grep -iqw $new_llvm_component ; then
> > +LLVM_COMPONENTS="${LLVM_COMPONENTS} ${new_llvm_component}"
> > +#else
> > +#AC_MSG_ERROR([LLVM component '$new_llvm_component' not enabled
> > in your LLVM build. Required by $driver_name.])
> This function adds required components, correct ? Then the above two
> lines should not be commented out.
I can't enable this until later in the series as mentioned in the commit
message.
This is how it worked before (there were no errors for the components).
At this point llvm_add_default_components will be added even if we don't need
LLVM. This way someone without LLVM installed will get error messages even
though he might just want to build the intel or softpipe driver.
It will be enabled in Patch 24.
>
> -Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 02/25] configure.ac: Add helper function for targets/components
On 12 October 2016 at 00:02, Tobias Droste wrote:
> Add functions to add and check targets/components.
> Not used in this patch.
>
> The error message in llvm_add_component is disabled until it doesn't break
> the build anymore.
> This is the same functionality as before where the components were added
> without a check.
>
> Signed-off-by: Tobias Droste
> ---
> configure.ac | 35 +++
> 1 file changed, 35 insertions(+)
>
> diff --git a/configure.ac b/configure.ac
> index bdd46bc..69421ff 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -2196,7 +2196,42 @@ llvm_check_version_for() {
> fi
> }
>
> +llvm_add_default_components() {
> +driver_name=$1
>
> +# Required default components
> +llvm_add_component "engine" $driver_name
> +llvm_add_component "bitwriter" $driver_name
> +llvm_add_component "mcjit" $driver_name
> +llvm_add_component "mcdisassembler" $driver_name
> +
> +# Optional default components
> +if $LLVM_CONFIG --components | grep -iqw inteljitevents ; then
> +LLVM_COMPONENTS="${LLVM_COMPONENTS} inteljitevents"
> +fi
> +}
> +
> +llvm_add_component() {
Nit: move this function before its user (llvm_add_default_components)
Idea: call this ...components (plural) and feed the all components at once.
> +new_llvm_component=$1
> +driver_name=$2
> +
> +if $LLVM_CONFIG --components | grep -iqw $new_llvm_component ; then
> +LLVM_COMPONENTS="${LLVM_COMPONENTS} ${new_llvm_component}"
> +#else
> +#AC_MSG_ERROR([LLVM component '$new_llvm_component' not enabled in
> your LLVM build. Required by $driver_name.])
This function adds required components, correct ? Then the above two
lines should not be commented out.
-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
