r212666 - [Driver] Expose getARMCPUForMArch() function in the Driver API; NFC.

[Eric Christopher]

On Thu, Jul 10, 2014 at 11:47 AM, Argyrios Kyrtzidis <akyrtzi at gmail.com> wrote:
>
>> On Jul 9, 2014, at 10:07 PM, Eric Christopher <echristo at gmail.com> 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 <akyrtzi at gmail.com> wrote:
>>> It is useful out-of-tree.
>>>
>>> On Jul 9, 2014, at 6:26 PM, Eric Christopher <echristo at gmail.com> wrote:
>>>
>>>> Why?
>>>>
>>>> -eric
>>>>
>>>> On Wed, Jul 9, 2014 at 6:03 PM, Argyrios Kyrtzidis <akyrtzi at gmail.com> 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
>>>>> cfe-commits at cs.uiuc.edu
>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>