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

Argyrios Kyrtzidis akyrtzi at gmail.com
Thu Jul 10 13:57:50 PDT 2014


> On Jul 10, 2014, at 12:46 PM, Chandler Carruth <chandlerc at google.com> wrote:
> 
> 
> On Thu, Jul 10, 2014 at 12:12 PM, Argyrios Kyrtzidis <akyrtzi at gmail.com> wrote:
> > 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.
> 
> 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.
> 
> Argiris, this is reasonable, minor, and easily addressed code review. I know you may have a lot of things on your plate, but *especially* when contributing patches purely to support out-of-tree targets that others don't know about, I think it is really important to promptly address any post-commit review you receive.
> 


Ok, will do.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140710/0d2ca0b1/attachment.html>


More information about the cfe-commits mailing list