r155431. On Tue, Apr 24, 2012 at 12:17 PM, James Molloy <[email protected]> wrote: >> 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! > > You mean you don't want me to commit this? > > Sorry I should have been more clear - I assumed the code has already been > committed, and thats why I wrote my response in that tense. > > I have no immediate problem with the code as-is - if other reviewers have > signed off on it then for sure commit it. I'll try and get a cleaner solution > together in the driver changes later. > > Cheers, > > James > > -----Original Message----- > From: Evgeniy Stepanov [mailto:[email protected]] > Sent: 24 April 2012 09:14 > 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 > > On Tue, Apr 24, 2012 at 12:10 PM, James Molloy <[email protected]> wrote: >> 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. > > Sounds great! > >> >> 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! > > You mean you don't want me to commit this? > >> >> 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
