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

[Eric Christopher]

Thanks Argyrios!

-eric

On Fri, Jul 11, 2014 at 4:57 PM, Argyrios Kyrtzidis <akyrtzi at gmail.com> wrote:
> See
> llvm r212846
> clang r212858
>
>> On Jul 10, 2014, at 11:51 AM, Eric Christopher <echristo at gmail.com> wrote:
>>
>> 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
>>>>>
>>>
>