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

Douglas Gregor dgregor at apple.com
Thu Jul 10 13:58:48 PDT 2014


> On Jul 10, 2014, at 1:57 PM, Argyrios Kyrtzidis <akyrtzi at gmail.com> wrote:
> 
> 
>> 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.


Thanks, Argyrios!

	- Doug
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140710/854b2fa5/attachment.html>


More information about the cfe-commits mailing list