diff mbox series

[Xen-devel,01/12] xen: clang: Support correctly cross-compile

Message ID 20190327184531.30986-2-julien.grall@arm.com
State New
Headers show
Series xen/arm: Add support to build with clang | expand

Commit Message

Julien Grall March 27, 2019, 6:45 p.m. UTC
Clang uses "-target" option for cross-compilation.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 config/StdGNU.mk | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Jan Beulich March 28, 2019, 9:55 a.m. UTC | #1
>>> On 27.03.19 at 19:45, <julien.grall@arm.com> wrote:
> Clang uses "-target" option for cross-compilation.

And all possible targets are always available? I'd like to point out
that CROSS_COMPILE can be used for other than actual cross
compilation, e.g. building with just an alternative tool chain
built for the same target. Dropping the $(CROSS_COMPILE)
prefixes makes this impossible afaict. Requiring suitable wrapper
scripts to be put in place would seem better to me.

I also wonder why this change is needed for Arm, but wasn't
needed so far for x86. But perhaps no-one ever tried using it
so far ...

> --- a/config/StdGNU.mk
> +++ b/config/StdGNU.mk
> @@ -1,8 +1,13 @@
>  AS         = $(CROSS_COMPILE)as
>  LD         = $(CROSS_COMPILE)ld
>  ifeq ($(clang),y)
> -CC         = $(CROSS_COMPILE)clang
> -CXX        = $(CROSS_COMPILE)clang++
> +ifneq ($(CROSS_COMPILE),)
> +CC         = clang -target $(CROSS_COMPILE:-=)
> +CXX        = clang++ -target $(CROSS_COMPILE:-=)

Is dropping dashes from the variable uniformly correct? If so,
could you please clarify in the commit message why that is?

Jan
Andrew Cooper March 28, 2019, 10:14 a.m. UTC | #2
On 28/03/2019 09:55, Jan Beulich wrote:
>>>> On 27.03.19 at 19:45, <julien.grall@arm.com> wrote:
>> Clang uses "-target" option for cross-compilation.
> And all possible targets are always available? I'd like to point out
> that CROSS_COMPILE can be used for other than actual cross
> compilation, e.g. building with just an alternative tool chain
> built for the same target. Dropping the $(CROSS_COMPILE)
> prefixes makes this impossible afaict. Requiring suitable wrapper
> scripts to be put in place would seem better to me.
>
> I also wonder why this change is needed for Arm, but wasn't
> needed so far for x86. But perhaps no-one ever tried using it
> so far ...

It seems that CROSS_COMPILE is a GNU-ism, which is not shared by the
clang world.  I can't find anything which will make you a
$FOO-$BAR-clang binary, whereas you do typically get clang-$X aliases
for the different versions of clang.

Using -target is from the Clang instructions on cross compilation, which
say to do it this way.  https://clang.llvm.org/docs/CrossCompilation.html

The targets supported will depend on the configuration Clang was
compiled with, but Clang specifically opposes GCC's way of requiring the
user to recompile for every different target.  It is expected that a
packager of clang will enable all of the supported targets in the
package they distribute.

~Andrew
Jan Beulich March 28, 2019, 10:28 a.m. UTC | #3
>>> On 28.03.19 at 11:14, <andrew.cooper3@citrix.com> wrote:
> On 28/03/2019 09:55, Jan Beulich wrote:
>>>>> On 27.03.19 at 19:45, <julien.grall@arm.com> wrote:
>>> Clang uses "-target" option for cross-compilation.
>> And all possible targets are always available? I'd like to point out
>> that CROSS_COMPILE can be used for other than actual cross
>> compilation, e.g. building with just an alternative tool chain
>> built for the same target. Dropping the $(CROSS_COMPILE)
>> prefixes makes this impossible afaict. Requiring suitable wrapper
>> scripts to be put in place would seem better to me.
>>
>> I also wonder why this change is needed for Arm, but wasn't
>> needed so far for x86. But perhaps no-one ever tried using it
>> so far ...
> 
> It seems that CROSS_COMPILE is a GNU-ism, which is not shared by the
> clang world.  I can't find anything which will make you a
> $FOO-$BAR-clang binary, whereas you do typically get clang-$X aliases
> for the different versions of clang.

Oh, interesting. For my own re-built tool chains I actually can't use
CROSS_COMPILE either, because traditionally all my wrapper scripts
have a suffix like you say clang uses too. I patch in

cross-compile ?= $(CROSS_COMPILE)$(1)

locally as a fallback, to then use it as

AS = $(call cross-compile,as)

and then override things in the build root directory .config to
suite my actual needs, e.g. in its simplest possible form

cross-compile=$(1)x

But of course this doesn't fit clang either, as it's (aiui) only the
compiler which wants to be overridden this way.

> Using -target is from the Clang instructions on cross compilation, which
> say to do it this way.  https://clang.llvm.org/docs/CrossCompilation.html 
> 
> The targets supported will depend on the configuration Clang was
> compiled with, but Clang specifically opposes GCC's way of requiring the
> user to recompile for every different target.  It is expected that a
> packager of clang will enable all of the supported targets in the
> package they distribute.

Are you sure a distro caring about, say, only x86 would indeed
enable Arm and all sorts of other architectures in the compiler,
just because it can be enabled? IOW I assume the need for an
override to the system default clang binaries would still exist.

Jan
Andrew Cooper March 28, 2019, 10:43 a.m. UTC | #4
On 28/03/2019 10:28, Jan Beulich wrote:
>>>> On 28.03.19 at 11:14, <andrew.cooper3@citrix.com> wrote:
>> On 28/03/2019 09:55, Jan Beulich wrote:
>>>>>> On 27.03.19 at 19:45, <julien.grall@arm.com> wrote:
>>>> Clang uses "-target" option for cross-compilation.
>>> And all possible targets are always available? I'd like to point out
>>> that CROSS_COMPILE can be used for other than actual cross
>>> compilation, e.g. building with just an alternative tool chain
>>> built for the same target. Dropping the $(CROSS_COMPILE)
>>> prefixes makes this impossible afaict. Requiring suitable wrapper
>>> scripts to be put in place would seem better to me.
>>>
>>> I also wonder why this change is needed for Arm, but wasn't
>>> needed so far for x86. But perhaps no-one ever tried using it
>>> so far ...
>> It seems that CROSS_COMPILE is a GNU-ism, which is not shared by the
>> clang world.  I can't find anything which will make you a
>> $FOO-$BAR-clang binary, whereas you do typically get clang-$X aliases
>> for the different versions of clang.
> Oh, interesting. For my own re-built tool chains I actually can't use
> CROSS_COMPILE either, because traditionally all my wrapper scripts
> have a suffix like you say clang uses too. I patch in
>
> cross-compile ?= $(CROSS_COMPILE)$(1)
>
> locally as a fallback, to then use it as
>
> AS = $(call cross-compile,as)
>
> and then override things in the build root directory .config to
> suite my actual needs, e.g. in its simplest possible form
>
> cross-compile=$(1)x
>
> But of course this doesn't fit clang either, as it's (aiui) only the
> compiler which wants to be overridden this way.
>
>> Using -target is from the Clang instructions on cross compilation, which
>> say to do it this way.  https://clang.llvm.org/docs/CrossCompilation.html 
>>
>> The targets supported will depend on the configuration Clang was
>> compiled with, but Clang specifically opposes GCC's way of requiring the
>> user to recompile for every different target.  It is expected that a
>> packager of clang will enable all of the supported targets in the
>> package they distribute.
> Are you sure a distro caring about, say, only x86 would indeed
> enable Arm and all sorts of other architectures in the compiler,
> just because it can be enabled? IOW I assume the need for an
> override to the system default clang binaries would still exist.

I've just tried, and Ubuntu 16.04's default clang-3.8 is perfectly happy
compiling Aarch64, and makes a suitable looking elf object.  (I can't
actually disassemble it because objdump chokes, but .text is the
expected length)

As the cross-compilation documentation states, this is a deliberate
design decision which, amongst other things, prevents distros from
needing to maintain per-arch packages.

~Andrew
Jan Beulich March 28, 2019, 10:56 a.m. UTC | #5
>>> On 28.03.19 at 11:43, <andrew.cooper3@citrix.com> wrote:
> On 28/03/2019 10:28, Jan Beulich wrote:
>>>>> On 28.03.19 at 11:14, <andrew.cooper3@citrix.com> wrote:
>>> Using -target is from the Clang instructions on cross compilation, which
>>> say to do it this way.  https://clang.llvm.org/docs/CrossCompilation.html 
>>>
>>> The targets supported will depend on the configuration Clang was
>>> compiled with, but Clang specifically opposes GCC's way of requiring the
>>> user to recompile for every different target.  It is expected that a
>>> packager of clang will enable all of the supported targets in the
>>> package they distribute.
>> Are you sure a distro caring about, say, only x86 would indeed
>> enable Arm and all sorts of other architectures in the compiler,
>> just because it can be enabled? IOW I assume the need for an
>> override to the system default clang binaries would still exist.
> 
> I've just tried, and Ubuntu 16.04's default clang-3.8 is perfectly happy
> compiling Aarch64, and makes a suitable looking elf object.  (I can't
> actually disassemble it because objdump chokes, but .text is the
> expected length)
> 
> As the cross-compilation documentation states, this is a deliberate
> design decision which, amongst other things, prevents distros from
> needing to maintain per-arch packages.

All understood, just that Ubuntu may not be a good example, as
there looks to be Ubuntu 16.04 for 64-bit Arm.

Jan
Julien Grall March 29, 2019, 9:41 a.m. UTC | #6
Hi,

On 28/03/2019 09:55, Jan Beulich wrote:
>>>> On 27.03.19 at 19:45, <julien.grall@arm.com> wrote:
>> Clang uses "-target" option for cross-compilation.
> 
>> --- a/config/StdGNU.mk
>> +++ b/config/StdGNU.mk
>> @@ -1,8 +1,13 @@
>>   AS         = $(CROSS_COMPILE)as
>>   LD         = $(CROSS_COMPILE)ld
>>   ifeq ($(clang),y)
>> -CC         = $(CROSS_COMPILE)clang
>> -CXX        = $(CROSS_COMPILE)clang++
>> +ifneq ($(CROSS_COMPILE),)
>> +CC         = clang -target $(CROSS_COMPILE:-=)
>> +CXX        = clang++ -target $(CROSS_COMPILE:-=)
> 
> Is dropping dashes from the variable uniformly correct? If so,
> could you please clarify in the commit message why that is?
The target option requires the following format:

<arch><sub>-<vendor>-<sys>-<abi>

In other places, we need the trailing dash as GNU tools are using the same 
format as above with a dash to separate the tool name.

Cheers,
Julien Grall March 29, 2019, 10:09 a.m. UTC | #7
Hi,

On 28/03/2019 10:56, Jan Beulich wrote:
>>>> On 28.03.19 at 11:43, <andrew.cooper3@citrix.com> wrote:
>> On 28/03/2019 10:28, Jan Beulich wrote:
>>>>>> On 28.03.19 at 11:14, <andrew.cooper3@citrix.com> wrote:
>>>> Using -target is from the Clang instructions on cross compilation, which
>>>> say to do it this way.  https://clang.llvm.org/docs/CrossCompilation.html
>>>>
>>>> The targets supported will depend on the configuration Clang was
>>>> compiled with, but Clang specifically opposes GCC's way of requiring the
>>>> user to recompile for every different target.  It is expected that a
>>>> packager of clang will enable all of the supported targets in the
>>>> package they distribute.
>>> Are you sure a distro caring about, say, only x86 would indeed
>>> enable Arm and all sorts of other architectures in the compiler,
>>> just because it can be enabled? IOW I assume the need for an
>>> override to the system default clang binaries would still exist.
>>
>> I've just tried, and Ubuntu 16.04's default clang-3.8 is perfectly happy
>> compiling Aarch64, and makes a suitable looking elf object.  (I can't
>> actually disassemble it because objdump chokes, but .text is the
>> expected length)
>>
>> As the cross-compilation documentation states, this is a deliberate
>> design decision which, amongst other things, prevents distros from
>> needing to maintain per-arch packages.
> 
> All understood, just that Ubuntu may not be a good example, as
> there looks to be Ubuntu 16.04 for 64-bit Arm.

It occurs to me that other compiler may be based on clang/llvm but use a 
different name. For instance, the Arm Compiler is called armclang.

armclang only supports arm32 and arm64 and require to always pass the target 
triple using --target (Not the -- rather than -).

So we would need to cater different clang binary in the future.

Cheers,
Jan Beulich March 29, 2019, 10:14 a.m. UTC | #8
>>> On 29.03.19 at 10:41, <julien.grall@arm.com> wrote:
> On 28/03/2019 09:55, Jan Beulich wrote:
>>>>> On 27.03.19 at 19:45, <julien.grall@arm.com> wrote:
>>> Clang uses "-target" option for cross-compilation.
>> 
>>> --- a/config/StdGNU.mk
>>> +++ b/config/StdGNU.mk
>>> @@ -1,8 +1,13 @@
>>>   AS         = $(CROSS_COMPILE)as
>>>   LD         = $(CROSS_COMPILE)ld
>>>   ifeq ($(clang),y)
>>> -CC         = $(CROSS_COMPILE)clang
>>> -CXX        = $(CROSS_COMPILE)clang++
>>> +ifneq ($(CROSS_COMPILE),)
>>> +CC         = clang -target $(CROSS_COMPILE:-=)
>>> +CXX        = clang++ -target $(CROSS_COMPILE:-=)
>> 
>> Is dropping dashes from the variable uniformly correct? If so,
>> could you please clarify in the commit message why that is?
> The target option requires the following format:
> 
> <arch><sub>-<vendor>-<sys>-<abi>
> 
> In other places, we need the trailing dash as GNU tools are using the same 
> format as above with a dash to separate the tool name.

Oh, I'm sorry - I keep forgetting that the substitution form you
use only fiddles with trailing dashes. Of course I won't insist, but
I'd prefer the more obvious $(patsubst %-,%,$(CROSS_COMPILE))
to be used instead, despite realizing that it's meaningfully longer.

Jan
Stefano Stabellini April 19, 2019, 6:47 p.m. UTC | #9
On Fri, 29 Mar 2019, Jan Beulich wrote:
> >>> On 29.03.19 at 10:41, <julien.grall@arm.com> wrote:
> > On 28/03/2019 09:55, Jan Beulich wrote:
> >>>>> On 27.03.19 at 19:45, <julien.grall@arm.com> wrote:
> >>> Clang uses "-target" option for cross-compilation.
> >> 
> >>> --- a/config/StdGNU.mk
> >>> +++ b/config/StdGNU.mk
> >>> @@ -1,8 +1,13 @@
> >>>   AS         = $(CROSS_COMPILE)as
> >>>   LD         = $(CROSS_COMPILE)ld
> >>>   ifeq ($(clang),y)
> >>> -CC         = $(CROSS_COMPILE)clang
> >>> -CXX        = $(CROSS_COMPILE)clang++
> >>> +ifneq ($(CROSS_COMPILE),)
> >>> +CC         = clang -target $(CROSS_COMPILE:-=)
> >>> +CXX        = clang++ -target $(CROSS_COMPILE:-=)
> >> 
> >> Is dropping dashes from the variable uniformly correct? If so,
> >> could you please clarify in the commit message why that is?
> > The target option requires the following format:
> > 
> > <arch><sub>-<vendor>-<sys>-<abi>
> > 
> > In other places, we need the trailing dash as GNU tools are using the same 
> > format as above with a dash to separate the tool name.
> 
> Oh, I'm sorry - I keep forgetting that the substitution form you
> use only fiddles with trailing dashes. Of course I won't insist, but
> I'd prefer the more obvious $(patsubst %-,%,$(CROSS_COMPILE))
> to be used instead, despite realizing that it's meaningfully longer.

Either way to do variable manipulation is OK, but it would be good to
add Julien's explanation of the -target expected format in the commit
message. I don't have any other comments on this patch.
Julien Grall April 24, 2019, 8:18 p.m. UTC | #10
Hi,

On 4/19/19 7:47 PM, Stefano Stabellini wrote:
> On Fri, 29 Mar 2019, Jan Beulich wrote:
>>>>> On 29.03.19 at 10:41, <julien.grall@arm.com> wrote:
>>> On 28/03/2019 09:55, Jan Beulich wrote:
>>>>>>> On 27.03.19 at 19:45, <julien.grall@arm.com> wrote:
>>>>> Clang uses "-target" option for cross-compilation.
>>>>
>>>>> --- a/config/StdGNU.mk
>>>>> +++ b/config/StdGNU.mk
>>>>> @@ -1,8 +1,13 @@
>>>>>    AS         = $(CROSS_COMPILE)as
>>>>>    LD         = $(CROSS_COMPILE)ld
>>>>>    ifeq ($(clang),y)
>>>>> -CC         = $(CROSS_COMPILE)clang
>>>>> -CXX        = $(CROSS_COMPILE)clang++
>>>>> +ifneq ($(CROSS_COMPILE),)
>>>>> +CC         = clang -target $(CROSS_COMPILE:-=)
>>>>> +CXX        = clang++ -target $(CROSS_COMPILE:-=)
>>>>
>>>> Is dropping dashes from the variable uniformly correct? If so,
>>>> could you please clarify in the commit message why that is?
>>> The target option requires the following format:
>>>
>>> <arch><sub>-<vendor>-<sys>-<abi>
>>>
>>> In other places, we need the trailing dash as GNU tools are using the same
>>> format as above with a dash to separate the tool name.
>>
>> Oh, I'm sorry - I keep forgetting that the substitution form you
>> use only fiddles with trailing dashes. Of course I won't insist, but
>> I'd prefer the more obvious $(patsubst %-,%,$(CROSS_COMPILE))
>> to be used instead, despite realizing that it's meaningfully longer.
> 
> Either way to do variable manipulation is OK, but it would be good to
> add Julien's explanation of the -target expected format in the commit
> message. I don't have any other comments on this patch.

Not even regarding <b4405caf-6b06-6066-8f48-f35ba67bbdd2@arm.com>?

Cheers,
diff mbox series

Patch

diff --git a/config/StdGNU.mk b/config/StdGNU.mk
index 039274ea61..48c50b5ad7 100644
--- a/config/StdGNU.mk
+++ b/config/StdGNU.mk
@@ -1,8 +1,13 @@ 
 AS         = $(CROSS_COMPILE)as
 LD         = $(CROSS_COMPILE)ld
 ifeq ($(clang),y)
-CC         = $(CROSS_COMPILE)clang
-CXX        = $(CROSS_COMPILE)clang++
+ifneq ($(CROSS_COMPILE),)
+CC         = clang -target $(CROSS_COMPILE:-=)
+CXX        = clang++ -target $(CROSS_COMPILE:-=)
+else
+CC         = clang
+CXX        = clang++
+endif
 LD_LTO     = $(CROSS_COMPILE)llvm-ld
 else
 CC         = $(CROSS_COMPILE)gcc