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

Argyrios Kyrtzidis akyrtzi at gmail.com
Fri Jul 11 16:57:17 PDT 2014


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
>>>> 
>> 





More information about the cfe-commits mailing list