> 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:eugeni.stepa...@gmail.com] Sent: 24 April 2012 09:14 To: James Molloy Cc: Hal Finkel; cfe-commits@cs.uiuc.edu 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 <james.mol...@arm.com> 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:eugeni.stepa...@gmail.com] > Sent: 24 April 2012 08:59 > To: James Molloy > Cc: Hal Finkel; cfe-commits@cs.uiuc.edu > 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 <james.mol...@arm.com> 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: cfe-commits-boun...@cs.uiuc.edu >> [mailto:cfe-commits-boun...@cs.uiuc.edu] On Behalf Of Hal Finkel >> Sent: 23 April 2012 15:08 >> To: Evgeniy Stepanov >> Cc: cfe-commits@cs.uiuc.edu >> 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 <eugeni.stepa...@gmail.com> 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 <hfin...@anl.gov> wrote: >>> > On Mon, 23 Apr 2012 07:14:19 -0500 >>> > Hal Finkel <hfin...@anl.gov> wrote: >>> > >>> >> On Mon, 23 Apr 2012 11:14:04 +0400 >>> >> Evgeniy Stepanov <eugeni.stepa...@gmail.com> wrote: >>> >> >>> >> > On Sun, Apr 22, 2012 at 5:41 AM, Hal Finkel <hfin...@anl.gov> >>> >> > 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 <eugeni.stepa...@gmail.com> 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 >>> >> > >> cfe-commits@cs.uiuc.edu >>> >> > >> 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 >> cfe-commits@cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> >> > > > _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
