> On Jul 10, 2014, at 11:51 AM, Eric Christopher <[email protected]> wrote: > > 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.
I appreciate the feedback but it will be low on my TODO list for API refactoring for this, if you or anyone else wants to refactor it feel free. > >> >>> 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
