[PATCH] D25564: Add interface to compute number of physical cores on host system

Aaron Ballman via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 13 10:41:31 PDT 2016


On Thu, Oct 13, 2016 at 1:39 PM, Teresa Johnson <tejohnson at google.com>
wrote:

>
>
> On Thu, Oct 13, 2016 at 10:35 AM, Aaron Ballman <aaron.ballman at gmail.com>
> wrote:
>
>> On Thu, Oct 13, 2016 at 1:30 PM, Mehdi AMINI <mehdi.amini at apple.com>
>> wrote:
>> > mehdi_amini added inline comments.
>> >
>> >
>> > ================
>> > Comment at: lib/Support/Host.cpp:1239
>> > +// On other systems, return -1 to indicate unknown.
>> > +int computeHostNumPhysicalCores() { return -1; }
>> > +#endif
>> > ----------------
>> > aaron.ballman wrote:
>> >> No Windows implementation? Should be able to use
>> `GetLogicalProcessorInformation()` pretty easily.
>> >>
>> >> https://msdn.microsoft.com/en-us/library/windows/desktop/ms6
>> 83194(v=vs.85).aspx
>> > Patch welcome :)
>>
>> Happy to work on a patch, but I worry when we add new interfaces that
>> don't work across platforms unless they bark loudly. An assert() here
>> might not be amiss.
>>
>
> I don't want to add an assert, because when I submit the follow-on patch
> that uses this in a new thread interface it will fall back to
> hardware_concurrency if this returns -1. This seems like a friendlier
> approach, since for all platforms but the ones implemented here it will
> cause the user (ThinLTO backends initially) to fall back to the current
> default parallelism. And they will then automatically pick up any support
> added for other platforms.
>

Okay, that's totally fair. Thank you for the explanation!

~Aaron


>
>
>>
>> ~Aaron
>>
>> >
>> > I'll supply the MacOS version when possible.
>> >
>> >
>> > https://reviews.llvm.org/D25564
>> >
>> >
>> >
>>
>
>
>
> --
> Teresa Johnson |  Software Engineer |  tejohnson at google.com |
>  408-460-2413
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161013/5c367a39/attachment.html>


More information about the llvm-commits mailing list