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

Chandler Carruth chandlerc at google.com
Thu Jul 10 12:46:30 PDT 2014

On Thu, Jul 10, 2014 at 12:12 PM, Argyrios Kyrtzidis <akyrtzi at gmail.com>

> > 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140710/1180f801/attachment.html>

More information about the cfe-commits mailing list