On Thu, Jul 10, 2014 at 11:47 AM, Argyrios Kyrtzidis <[email protected]> wrote: > >> On Jul 9, 2014, at 10:07 PM, Eric Christopher <[email protected]> wrote: >> >> Seems like an odd API to expose out of the driver. Perhaps move it to >> Triple and do something there? Other API choices? > > Since this was implemented in the clang driver I don’t see a compelling > reason to move it but I have no objection either.
It was implemented as a helper function, not a formal API in the driver. It's not a driver level function and you're basically exposing a code generation function as part of the driver. It doesn't make a lot of sense there. That's why I was suggesting in the support library alongside Triple. We already have a dependence upon Triple in clang so we wouldn't be adding anything new. If you want this exported I think that's the best place to do so. > >> Also unless there's >> a test on it then someone might end up just re-inlining it where >> you've got it separated :) > > Added a unit test in r212751, thanks for the feedback! Welcome. -eric > >> >> -eric >> >> On Wed, Jul 9, 2014 at 10:05 PM, Argyrios Kyrtzidis <[email protected]> >> wrote: >>> It is useful out-of-tree. >>> >>> On Jul 9, 2014, at 6:26 PM, Eric Christopher <[email protected]> wrote: >>> >>>> Why? >>>> >>>> -eric >>>> >>>> On Wed, Jul 9, 2014 at 6:03 PM, Argyrios Kyrtzidis <[email protected]> >>>> wrote: >>>>> Author: akirtzidis >>>>> Date: Wed Jul 9 20:03:37 2014 >>>>> New Revision: 212666 >>>>> >>>>> URL: http://llvm.org/viewvc/llvm-project?rev=212666&view=rev >>>>> Log: >>>>> [Driver] Expose getARMCPUForMArch() function in the Driver API; NFC. >>>>> >>>>> Modified: >>>>> cfe/trunk/include/clang/Driver/Util.h >>>>> cfe/trunk/lib/Driver/Tools.cpp >>>>> >>>>> Modified: cfe/trunk/include/clang/Driver/Util.h >>>>> URL: >>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Util.h?rev=212666&r1=212665&r2=212666&view=diff >>>>> ============================================================================== >>>>> --- cfe/trunk/include/clang/Driver/Util.h (original) >>>>> +++ cfe/trunk/include/clang/Driver/Util.h Wed Jul 9 20:03:37 2014 >>>>> @@ -13,6 +13,10 @@ >>>>> #include "clang/Basic/LLVM.h" >>>>> #include "llvm/ADT/DenseMap.h" >>>>> >>>>> +namespace llvm { >>>>> + class Triple; >>>>> +} >>>>> + >>>>> namespace clang { >>>>> class DiagnosticsEngine; >>>>> >>>>> @@ -26,6 +30,9 @@ namespace driver { >>>>> /// ActionList - Type used for lists of actions. >>>>> typedef SmallVector<Action*, 3> ActionList; >>>>> >>>>> +/// Get the (LLVM) name of the minimum ARM CPU for the arch we are >>>>> targeting. >>>>> +const char* getARMCPUForMArch(StringRef MArch, const llvm::Triple >>>>> &Triple); >>>>> + >>>>> } // end namespace driver >>>>> } // end namespace clang >>>>> >>>>> >>>>> Modified: cfe/trunk/lib/Driver/Tools.cpp >>>>> URL: >>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=212666&r1=212665&r2=212666&view=diff >>>>> ============================================================================== >>>>> --- cfe/trunk/lib/Driver/Tools.cpp (original) >>>>> +++ cfe/trunk/lib/Driver/Tools.cpp Wed Jul 9 20:03:37 2014 >>>>> @@ -5025,9 +5025,6 @@ void hexagon::Link::ConstructJob(Compila >>>>> } >>>>> // Hexagon tools end. >>>>> >>>>> -/// getARMCPUForMArch - Get the (LLVM) name of the minimum ARM CPU for >>>>> the arch we are targeting >>>>> -// >>>>> -// FIXME: tblgen this. >>>>> const char *arm::getARMCPUForMArch(const ArgList &Args, >>>>> const llvm::Triple &Triple) { >>>>> StringRef MArch; >>>>> @@ -5049,6 +5046,14 @@ const char *arm::getARMCPUForMArch(const >>>>> } >>>>> } >>>>> >>>>> + return driver::getARMCPUForMArch(MArch, Triple); >>>>> +} >>>>> + >>>>> +/// Get the (LLVM) name of the minimum ARM CPU for the arch we are >>>>> targeting. >>>>> +// >>>>> +// FIXME: tblgen this. >>>>> +const char *driver::getARMCPUForMArch(StringRef MArch, >>>>> + const llvm::Triple &Triple) { >>>>> switch (Triple.getOS()) { >>>>> case llvm::Triple::NetBSD: >>>>> if (MArch == "armv6") >>>>> >>>>> >>>>> _______________________________________________ >>>>> 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
