I think we should make sure the "universal driver" can deal with this, and 
remove the functionality then.

I'm actively working on this now (I was pre-February at which point I had a 
brief hiatus for the LLVM conference. Back on again.), and have a patch 
outstanding on the driver.

The problem with testing is a difficult one, but one I'm going to try and solve 
by building up a database of known driver behaviour - inputs and outputs - for 
both Clang in its current state and GCC on as many different platforms as I can 
find (I will be asking the community to help with this at some point).

>From there I can create a test suite that should run on any platform and test 
>all platforms. The ideal goal being that the driver is "known to be as correct 
>as GCC" on supported platforms, and easy to extend for nonsupported platforms.

For the moment, I think this commit should stay in place as-is, there's no 
point expending extra effort on it I don't think!

Cheers,

James

-----Original Message-----
From: Evgeniy Stepanov [mailto:[email protected]] 
Sent: 24 April 2012 08:59
To: James Molloy
Cc: Hal Finkel; [email protected]
Subject: Re: [cfe-commits] r154389 - in /cfe/trunk: lib/Driver/Tools.cpp 
test/Driver/linux-as.c

Not a hack, but unimplemented functionality.
Ideally, every other platform should also have a set of forwarded (and
maybe transformed) options, but I'm reluctant to do this now. I can't
test it easily, and I don't know enough about most of them to be sure
that I don't screw something up.
WDYT?


On Mon, Apr 23, 2012 at 6:12 PM, James Molloy <[email protected]> wrote:
> It still sounds like a pretty dirty hack to get something to work in the 
> interim.
>
> People will wonder why passing -mcpu only works for ARM and not for any other 
> platform...
>
> -----Original Message-----
> From: [email protected] 
> [mailto:[email protected]] On Behalf Of Hal Finkel
> Sent: 23 April 2012 15:08
> To: Evgeniy Stepanov
> Cc: [email protected]
> Subject: Re: [cfe-commits] r154389 - in /cfe/trunk: lib/Driver/Tools.cpp 
> test/Driver/linux-as.c
>
> LGTM, thanks!
>
>  -Hal
>
> On Mon, 23 Apr 2012 16:53:41 +0400
> Evgeniy Stepanov <[email protected]> wrote:
>
>> Ok, I moved this options under if(arm).
>> Please take a look at the patch.
>>
>> On Mon, Apr 23, 2012 at 4:35 PM, Hal Finkel <[email protected]> wrote:
>> > On Mon, 23 Apr 2012 07:14:19 -0500
>> > Hal Finkel <[email protected]> wrote:
>> >
>> >> On Mon, 23 Apr 2012 11:14:04 +0400
>> >> Evgeniy Stepanov <[email protected]> wrote:
>> >>
>> >> > On Sun, Apr 22, 2012 at 5:41 AM, Hal Finkel <[email protected]>
>> >> > wrote:
>> >> > > Evgeniy,
>> >> > >
>> >> > > Unfortunately, we need to be much more careful here. While gas
>> >> > > for ARM does support -mcpu, -march and -mfpu, this is not true
>> >> > > for other ISAs. Looking at the man page, it seems that the
>> >> > > supported flags are:
>> >> > >
>> >> > > ARM: -mcpu=, -march=, -mfpu=
>> >> > > i386: -march= (-mtune=: should we map -mcpu= to -mtune=?)
>> >> > > MIPS: (same as i386)
>> >> > > PowerPC: supports cpu, but as -m<cpu> (meaning
>> >> > > -m403|-m405|-mppc64|...) SPARC: supports arch, but as -xarch=
>> >> > >
>> >> > > How do you think we should fix this?
>> >> >
>> >> > Well, we can make the set of forwarded options dependent on the
>> >> > target.
>> >>
>> >> That makes sense.
>> >>
>> >> >
>> >> > But what's the failing use case? I mean, why do you pass these
>> >> > options to the compiler in the first place?
>> >>
>> >> This patch broke building on PPC with a non-default cpu type. I
>> >> pass -mcpu=, as you do, to select appropriate cpu features and
>> >> itineraries. The default clang behavior on PPC is to pass -many to
>> >> the assembler. As you might imagine, this enables instructions for
>> >> all known cpu types. With this patch, the assembler also gets the
>> >> -mcpu=<cpu> flag provided to the driver. This is incorrect, it
>> >> would need to be translated as -m<cpu> for PPC. I think it is
>> >> unfortunate that gas cannot accept a common set of command-line
>> >> parameters across platforms, but that I cannot fix ;)
>> >
>> > I should also add that it is quite possible for LLVM to have
>> > itineraries for cpus that gas knows nothing about in particular.
>> >
>> >  -Hal
>> >
>> >>
>> >> Thanks again,
>> >> Hal
>> >>
>> >> >
>> >> > >
>> >> > >  -Hal
>> >> > >
>> >> > > On Tue, 10 Apr 2012 09:05:41 -0000
>> >> > > Evgeniy Stepanov <[email protected]> wrote:
>> >> > >
>> >> > >> Author: eugenis
>> >> > >> Date: Tue Apr 10 04:05:40 2012
>> >> > >> New Revision: 154389
>> >> > >>
>> >> > >> URL: http://llvm.org/viewvc/llvm-project?rev=154389&view=rev
>> >> > >> Log:
>> >> > >> Pass -march, -mcpu, -mfpu to linuxtools assembler.
>> >> > >>
>> >> > >> Added:
>> >> > >>     cfe/trunk/test/Driver/linux-as.c
>> >> > >> Modified:
>> >> > >>     cfe/trunk/lib/Driver/Tools.cpp
>> >> > >>
>> >> > >> Modified: cfe/trunk/lib/Driver/Tools.cpp
>> >> > >> URL:
>> >> > >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=154389&r1=154388&r2=154389&view=diff
>> >> > >> ==============================================================================
>> >> > >> --- cfe/trunk/lib/Driver/Tools.cpp (original) +++
>> >> > >> cfe/trunk/lib/Driver/Tools.cpp Tue Apr 10 04:05:40 2012 @@
>> >> > >> -5089,6 +5089,10 @@ CmdArgs.push_back("-EL");
>> >> > >>    }
>> >> > >>
>> >> > >> +  Args.AddLastArg(CmdArgs, options::OPT_march_EQ);
>> >> > >> +  Args.AddLastArg(CmdArgs, options::OPT_mcpu_EQ);
>> >> > >> +  Args.AddLastArg(CmdArgs, options::OPT_mfpu_EQ);
>> >> > >> +
>> >> > >>    Args.AddAllArgValues(CmdArgs, options::OPT_Wa_COMMA,
>> >> > >>                         options::OPT_Xassembler);
>> >> > >>
>> >> > >>
>> >> > >> Added: cfe/trunk/test/Driver/linux-as.c
>> >> > >> URL:
>> >> > >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/linux-as.c?rev=154389&view=auto
>> >> > >> ==============================================================================
>> >> > >> --- cfe/trunk/test/Driver/linux-as.c (added) +++
>> >> > >> cfe/trunk/test/Driver/linux-as.c Tue Apr 10 04:05:40 2012 @@
>> >> > >> -0,0 +1,31 @@ +// Check passing options to the assembler for
>> >> > >> ARM targets. +//
>> >> > >> +// RUN: %clang -target arm-linux -### \
>> >> > >> +// RUN:   -no-integrated-as -c %s 2>&1 \
>> >> > >> +// RUN:   | FileCheck -check-prefix=ARM %s
>> >> > >> +// CHECK-ARM: as{{(.exe)?}}"
>> >> > >> +//
>> >> > >> +// RUN: %clang -target arm-linux -mcpu=cortex-a8 -### \
>> >> > >> +// RUN:   -no-integrated-as -c %s 2>&1 \
>> >> > >> +// RUN:   | FileCheck -check-prefix=ARM-MCPU %s
>> >> > >> +// CHECK-ARM-MCPU: as{{(.exe)?}}" "-mcpu=cortex-a8"
>> >> > >> +//
>> >> > >> +// RUN: %clang -target arm-linux -mfpu=neon -### \
>> >> > >> +// RUN:   -no-integrated-as -c %s 2>&1 \
>> >> > >> +// RUN:   | FileCheck -check-prefix=ARM-MFPU %s
>> >> > >> +// CHECK-ARM-MFPU: as{{(.exe)?}}" "-mfpu=neon"
>> >> > >> +//
>> >> > >> +// RUN: %clang -target arm-linux -march=armv7-a -### \
>> >> > >> +// RUN:   -no-integrated-as -c %s 2>&1 \
>> >> > >> +// RUN:   | FileCheck -check-prefix=ARM-MARCH %s
>> >> > >> +// CHECK-ARM-MARCH: as{{(.exe)?}}" "-march=armv7-a"
>> >> > >> +//
>> >> > >> +// RUN: %clang -target arm-linux -mcpu=cortex-a8 -mfpu=neon
>> >> > >> -march=armv7-a -### \ +// RUN:   -no-integrated-as -c %s 2>&1
>> >> > >> \ +// RUN:   | FileCheck -check-prefix=ARM-ALL %s
>> >> > >> +// CHECK-ARM-ALL: as{{(.exe)?}}" "-march=armv7-a"
>> >> > >> "-mcpu=cortex-a8" "-mfpu=neon" +//
>> >> > >> +// RUN: %clang -target armv7-linux -mcpu=cortex-a8 -### \
>> >> > >> +// RUN:   -no-integrated-as -c %s 2>&1 \
>> >> > >> +// RUN:   | FileCheck -check-prefix=ARM-TARGET %s
>> >> > >> +// CHECK-ARM-TARGET: as{{(.exe)?}}" "-mfpu=neon"
>> >> > >> "-mcpu=cortex-a8"
>> >> > >>
>> >> > >>
>> >> > >> _______________________________________________
>> >> > >> cfe-commits mailing list
>> >> > >> [email protected]
>> >> > >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>> >> > >
>> >> > >
>> >> > >
>> >> > > --
>> >> > > Hal Finkel
>> >> > > Postdoctoral Appointee
>> >> > > Leadership Computing Facility
>> >> > > Argonne National Laboratory
>> >>
>> >>
>> >>
>> >
>> >
>> >
>> > --
>> > Hal Finkel
>> > Postdoctoral Appointee
>> > Leadership Computing Facility
>> > Argonne National Laboratory
>
>
>
> --
> Hal Finkel
> Postdoctoral Appointee
> Leadership Computing Facility
> Argonne National Laboratory
>
> _______________________________________________
> cfe-commits mailing list
> [email protected]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>




_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to