Re: [wwwdocs] Document Runtime CPU detection builtins

[Sriraman Tallam <tmsriram at google dot com>]

Hi Gerald / Diego,

    I have made all the mentioned changes.  I also shortened the
description like Diego mentioned by removing all the strings but kept
the caveats. I have not added a reference to the documentation because
i do not know what link to reference. The builtins are completely
documented in extend.texi.

   I have attached the patch. If there are no further comments I will
submit this tomorrow.

Thanks,
-Sri.


On Mon, Aug 20, 2012 at 1:31 PM, Gerald Pfeifer <gerald@pfeifer.com> wrote:
> Hi Sriraman,
>
> On Fri, 10 Aug 2012, Sriraman Tallam wrote:
>> I have added a release note for x86 builtins __builtin_cpu_is and
>> __builtin_cpu_supports. They were checked in to trunk in rev. 186789.
>
> I had hoped one of the x86 maintainers would review this from his
> perspective given that they have more background.  For the lack of
> that, let me give it a try.
>
> Index: changes.html
> ===================================================================
> +    <li> New builtin functions to detect run-time CPU type and ISA:<br>
>
> "built-in", cf. http://gcc.gnu.org/codingconventions.html; here and
> in the following.
>
> No <br> here; <ul> should just do that.
>
> +    <ul>
> +      <li>Builtin <code>__builtin_cpu_is</code> has been added to detect if
> +      the run-time CPU is of a particular type. The builtin returns a postive
> +      integer on a match and zero otherwise. The builtin accepts one string
> +      literal argument, the CPU name. For example,
>
> "A built-in function..."
>
> "positive"
>
> "It accepts one string" (to make this shorter)
>
> +      <code>__builtin_cpu_is("westmere")</code> returns a postive integer if
>
> "positive"
>
> +      the run-time CPU is an Intel Corei7 Westmere processor.  The following
>
> I don't work for Intel, but should there be a space before "i7"?
>
> +      are the CPU names recognized by <code>__builtin_cpu_is:</code>
>
> How about making this "The following are the CPU names recognized for
> now", which avoids another reference to the name of the built-in and
> makes it clear that this is subject to change.
>
> +      <li>Builtin <code>__builtin_cpu_supports</code> has been added to detect
>
> "A built-in function..."
>
> +      returns a postive integer on a match and zero otherwise. The builtin
>
> "positive"
>
> +      following are the ISA features recognized by
> +      <code>__builtin_cpu_supports:</code>
>
> Same is above?
>
> +    <p>Caveat: If the above builtins are called before any constructors are
> +    invoked, like during IFUNC initialization, then the CPU detection
> +    initialization must be explicity run using this newly provided
> +    builtin,  <code>__builtin_cpu_init</code>.
>
> "...using the new built-in function <code>__builtin_cpu_init</code>."
>
> What is a constructor in this context, by the way?  Will this be clear
> to all the users?
>
> +    <code>
> +    static void (*some_ifunc_resolver(void))(void)<br>
> +    {<br>
> +    &nbsp&nbsp __builtin_cpu_init();<br>
> +    &nbsp&nbsp if (__builtin_cpu_is("amdfam10h") ...<br>
> +    &nbsp&nbsp if (__builtin_cpu_supports("popcnt") ...<br>
> +    }
> +    </code>
>
> How about using <pre> here? That avoids the <br/>s which will cause
> problems with the web page validator, by the way.
>
>
> Nice job for documenting this so well.  Thanks for taking the time
> and your patience!
>
> The patch is fine modulo the changes I pointed out (though some of
> them are more suggestions and you do not need to slavishly follow
> those).
>
> Gerald

Attachment: changes_html_patch.txt
Description: Text document