I'm ok with this patch. That can go to master, not need for api-next.
Maxim.
On 04/06/17 20:09, Brian Brooks wrote:
> On 04/05 01:24:44, Dmitry Eremin-Solenikov wrote:
>> On 04.04.2017 23:34, Brian Brooks wrote:
>>> On 04/04 23:27:51, Dmitry Eremin-Solenikov wrote:
>>>> On 04.04.2017 23:26, Brian Brooks wrote:
>>>>> On 04/04 23:04:10, Dmitry Eremin-Solenikov wrote:
>>>>>> On 04.04.2017 21:47, Brian Brooks wrote:
>>>>>>> Signed-off-by: Brian Brooks <brian.bro...@arm.com>
>>>>>>
>>>>>> Brian,
>>>>>>
>>>>>> how does this fail with clang on ARMv8? Could you please include error
>>>>>> message in the commit message?
>>>>>
>>>>> Well, you can't pass -mcx16 to clang when compiling natively on ARM.
>>>>>
>>>>> This is what it will look like:
>>>>>
>>>>> clang: error: argument unused during compilation: '-mcx16'
>>>>>
>>>>> and Autotools will halt the rest of the build.
>>>>
>>>> Does this happen during configure stage or when building?
>>>
>>> You will see that error during build. That is when clang is invoked.
>>> But, the bug is in configure.ac where -mcx16 is added to CFLAGS when
>>> it shouldn't be.
>>
>> Then why isn't it detected by configure script?
>
> The configure script is generated by Autoconf. Autoconf takes the
> configure.ac file as input. The autoconf tool is invoked when you
> run this project's ./bootstrap script.
>
> configure.ac is a combination of shell script and M4 (a macro language).
> If something in configure.ac doesn't look like shell, it is likely
> a macro.
>
> Lets step through it:
>
> 0 if test "${CC}" != "gcc" -o ${CC_VERSION_MAJOR} -ge 5; then
> 1 my_save_cflags="$CFLAGS"
> 2
> 3 CFLAGS=-mcx16
> 4 AC_MSG_CHECKING([whether CC supports -mcx16])
> 5 AC_COMPILE_IFELSE([AC_LANG_PROGRAM([])],
> 6 [AC_MSG_RESULT([yes])]
> 7 [ODP_CFLAGS="$ODP_CFLAGS $CFLAGS"],
> 8 [AC_MSG_RESULT([no])]
> 9 )
> 10 CFLAGS="$my_save_cflags"
> 11 fi
>
> Line 0 is specifically checking for whether we are using GCC or
> whether whatever compiler we're using's version is greater than
> or equal to 5.
>
> Line 1 saves CFLAGS into a temporary variable, and line 10 restores
> it. This technique is useful for when you want to avoid altering
> CFLAGS until the very end of the configure.ac has been processed.
> The reason is that the compiler may be invoked as configure.ac is
> processed and you want a pristine set of CFLAGS when you do this.
>
> Line 3 adds -mcx16 to CFLAGS. The original author probably wants
> to compile a small program with -mcx16 at this point in the
> processing of the configure.ac file. If we know we're not running
> on an x86-based chip, we can skip this entirely. More on that later.
>
> Line 4 is a macro to print something to inform the user that we
> are checking "whether CC supports -mcx16".
>
> Lines 5-9 will invoke the compiler with a provided program
> (an empty program: AC_LANG_PROGRAM([])) and execute lines 6 and 7
> on success or lines 7 and 8 on failure. The interesting part is
> line 7 which appends CFLAGS to ODP_CFLAGS on success. We know
> that CFLAGS contains -mcx16, but we must also be mindful that
> there is nothing else in CFLAGS otherwise multiple consecutive
> uses of this technique would end up appending redundant things
> to CFLAGS.
>
> So, what happens when the configure script is run on ARM with
> CC=clang?
>
> checking whether CC supports -mcx16... yes
>
> Wat... umm... let's try something else...
>
> $ touch poof.c
> $ clang -mcx16 poof.c
> clang: warning: argument unused during compilation: '-mcx16'
> (.text+0x30): undefined reference to `main'
> clang: error: linker command failed with exit code 1 (use -v to see
> invocation)
>
> OK. So, why does it fail when we do a 'make'? Because of -Werror.
>
> $ clang -Werror -mcx16 poof.c
> clang: error: argument unused during compilation: '-mcx16'
>
> -Werror is in ODP_CFLAGS, not CFLAGS used throughout the processing
> of the configure.ac file.
>
> An understandable argument is to add that flag to CFLAGS, but we can
> do better. Much better. We can skip this onzin entirely because we
> already know the architecture that we're targeting (the ${host}
> variable). Skipping that saves time.
>
> So, we only check for the x86-based -mcx16 flag if we're targeting
> an x86-based processor:
>
> if "${host}" == i?86* -o "${host}" == x86*; then
> if test "${CC}" != "gcc" -o ${CC_VERSION_MAJOR} -ge 5; then
>
> [..]
>
> fi
> fi
>
> It is that simple! :)
>
>> --
>> With best wishes
>> Dmitry
