Re: [Mesa-dev] [PATCH] configure.ac: Respect LLVM_CFLAGS in LLVM version detection

[Tomasz Figa]

On Mon, Dec 19, 2016 at 8:06 PM, Emil Velikov  wrote:
> On 19 December 2016 at 08:52, Tomasz Figa  wrote:
>> Hi Tobias,
>>
>> On Sat, Dec 17, 2016 at 2:15 AM, Tobias Droste  wrote:
>>> Hi Tomasz,
>>>
>>> does this actually fix anything?
>>>
>>> Because right now llvm-config.h does not include anything and I doubt it 
>>> will
>>> in the future, as it's just a collection of defines.
>>> The path to the header file itself is given by llvm-config 
>>> ($LLVM_INCLUDEDIR).
>>>
>>> Did you just happen to see this or do you get an error without this patch?
>>
>> We happen to have the setup exactly as described in the patch
>> description, i.e. LLVM in a non-standard directory and with
>> llvm-config.h that includes another header. Without the patch
>> ./configure fails because of LLVM version detection errors.
>>
> I believe you're spot on - be that due to missing include [directives]
> or conditional header inclusion one would need to have the respective
> CFLAGS. Thus the patch is
> Reviewed-by: Emil Velikov 
>
> At the same time - I seems to be lucky enough to have the headers sane
> [neither includes nor defines are required] on my Arch setup.
> Wondering if it's not something specific to the Android way of
> building LLVM

Yeah, that's very likely. :)

Although I can imagine distributors creating a wrapper including one
or another real header, depending on some setting. I might have seen
something like this in Gentoo, but I think it used absolute paths,
which made it compile without any flags.

> - IIRC the Android-x86 guys were explaining some
> interesting things.
> ... but that for another day. I'll commit your patch once I
> double-check Tobias' fixes.

Thanks!

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

Re: [Mesa-dev] [PATCH] configure.ac: Respect LLVM_CFLAGS in LLVM version detection

[Emil Velikov]

On 19 December 2016 at 08:52, Tomasz Figa  wrote:
> Hi Tobias,
>
> On Sat, Dec 17, 2016 at 2:15 AM, Tobias Droste  wrote:
>> Hi Tomasz,
>>
>> does this actually fix anything?
>>
>> Because right now llvm-config.h does not include anything and I doubt it will
>> in the future, as it's just a collection of defines.
>> The path to the header file itself is given by llvm-config 
>> ($LLVM_INCLUDEDIR).
>>
>> Did you just happen to see this or do you get an error without this patch?
>
> We happen to have the setup exactly as described in the patch
> description, i.e. LLVM in a non-standard directory and with
> llvm-config.h that includes another header. Without the patch
> ./configure fails because of LLVM version detection errors.
>
I believe you're spot on - be that due to missing include [directives]
or conditional header inclusion one would need to have the respective
CFLAGS. Thus the patch is
Reviewed-by: Emil Velikov 

At the same time - I seems to be lucky enough to have the headers sane
[neither includes nor defines are required] on my Arch setup.
Wondering if it's not something specific to the Android way of
building LLVM - IIRC the Android-x86 guys were explaining some
interesting things.
... but that for another day. I'll commit your patch once I
double-check Tobias' fixes.

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

Re: [Mesa-dev] [PATCH] configure.ac: Respect LLVM_CFLAGS in LLVM version detection

[Tomasz Figa]

Hi Tobias,

On Sat, Dec 17, 2016 at 2:15 AM, Tobias Droste  wrote:
> Hi Tomasz,
>
> does this actually fix anything?
>
> Because right now llvm-config.h does not include anything and I doubt it will
> in the future, as it's just a collection of defines.
> The path to the header file itself is given by llvm-config ($LLVM_INCLUDEDIR).
>
> Did you just happen to see this or do you get an error without this patch?

We happen to have the setup exactly as described in the patch
description, i.e. LLVM in a non-standard directory and with
llvm-config.h that includes another header. Without the patch
./configure fails because of LLVM version detection errors.

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

Re: [Mesa-dev] [PATCH] configure.ac: Respect LLVM_CFLAGS in LLVM version detection

[Tobias Droste]

Hi Tomasz,

does this actually fix anything?

Because right now llvm-config.h does not include anything and I doubt it will 
in the future, as it's just a collection of defines.
The path to the header file itself is given by llvm-config ($LLVM_INCLUDEDIR).

Did you just happen to see this or do you get an error without this patch?

Tobias

Am Mittwoch, 14. Dezember 2016, 17:00:27 CET schrieb Tomasz Figa:
> When compiling LLVM headers, including llvm-config.h, we need to respect
> LLVM_CFLAGS. This is especially crucial if LLVM is located in a
> non-standard location and it happens that llvm-config.h includes another
> header. In such case the detection would fail due to missing header,
> because the path is provided in LLVM_CFLAGS.
> 
> Let's add LLVM_CFLAGS to global CFLAGS for the time of detection and then
> restore the original flags, as done in other places of the script.
> 
> Signed-off-by: Tomasz Figa 
> ---
>  configure.ac | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/configure.ac b/configure.ac
> index 799f5eb..8fd7120 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -978,6 +978,10 @@ llvm_set_environment_variables() {
>  LLVM_INCLUDEDIR=`$LLVM_CONFIG --includedir`
>  LLVM_LIBDIR=`$LLVM_CONFIG --libdir`
> 
> +# We need to respect LLVM_CFLAGS when compiling LLVM headers.
> +save_CFLAGS="$CFLAGS"
> +CFLAGS="$CFLAGS $LLVM_CFLAGS"
> +
>  AC_COMPUTE_INT([LLVM_VERSION_MAJOR], [LLVM_VERSION_MAJOR],
>  [#include "${LLVM_INCLUDEDIR}/llvm/Config/llvm-config.h"])
>  AC_COMPUTE_INT([LLVM_VERSION_MINOR], [LLVM_VERSION_MINOR],
> @@ -985,6 +989,8 @@ llvm_set_environment_variables() {
>  AC_COMPUTE_INT([LLVM_VERSION_PATCH], [LLVM_VERSION_PATCH],
>  [#include "${LLVM_INCLUDEDIR}/llvm/Config/llvm-config.h"])
> 
> +CFLAGS="$save_CFLAGS"
> +
>  # Only needed for LLVM < 3.6.0
>  if test -z "$LLVM_VERSION_PATCH"; then
>  LLVM_VERSION_PATCH=0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] configure.ac: Respect LLVM_CFLAGS in LLVM version detection

[Tomasz Figa]

When compiling LLVM headers, including llvm-config.h, we need to respect
LLVM_CFLAGS. This is especially crucial if LLVM is located in a
non-standard location and it happens that llvm-config.h includes another
header. In such case the detection would fail due to missing header,
because the path is provided in LLVM_CFLAGS.

Let's add LLVM_CFLAGS to global CFLAGS for the time of detection and then
restore the original flags, as done in other places of the script.

Signed-off-by: Tomasz Figa 
---
 configure.ac | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/configure.ac b/configure.ac
index 799f5eb..8fd7120 100644
--- a/configure.ac
+++ b/configure.ac
@@ -978,6 +978,10 @@ llvm_set_environment_variables() {
 LLVM_INCLUDEDIR=`$LLVM_CONFIG --includedir`
 LLVM_LIBDIR=`$LLVM_CONFIG --libdir`
 
+# We need to respect LLVM_CFLAGS when compiling LLVM headers.
+save_CFLAGS="$CFLAGS"
+CFLAGS="$CFLAGS $LLVM_CFLAGS"
+
 AC_COMPUTE_INT([LLVM_VERSION_MAJOR], [LLVM_VERSION_MAJOR],
 [#include "${LLVM_INCLUDEDIR}/llvm/Config/llvm-config.h"])
 AC_COMPUTE_INT([LLVM_VERSION_MINOR], [LLVM_VERSION_MINOR],
@@ -985,6 +989,8 @@ llvm_set_environment_variables() {
 AC_COMPUTE_INT([LLVM_VERSION_PATCH], [LLVM_VERSION_PATCH],
 [#include "${LLVM_INCLUDEDIR}/llvm/Config/llvm-config.h"])
 
+CFLAGS="$save_CFLAGS"
+
 # Only needed for LLVM < 3.6.0
 if test -z "$LLVM_VERSION_PATCH"; then
 LLVM_VERSION_PATCH=0
-- 
2.8.0.rc3.226.g39d4020

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