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 <ger...@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
Index: changes.html
===================================================================
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.8/changes.html,v
retrieving revision 1.10
diff -u -r1.10 changes.html
--- changes.html        10 Aug 2012 16:25:46 -0000      1.10
+++ changes.html        21 Aug 2012 02:38:40 -0000
@@ -92,6 +92,38 @@
     wrong results.  You must build all
     modules with <code>-mpreferred-stack-boundary=3</code>, including any
     libraries.  This includes the system libraries and startup modules.</li>
+    <li> New built-in functions to detect run-time CPU type and ISA:
+    <ul>
+      <li>A built-in function <code>__builtin_cpu_is</code> has been added to
+      detect if the run-time CPU is of a particular type.  It returns a
+      positive integer on a match and zero otherwise.  It accepts one string
+      literal argument, the CPU name.  For example,
+      <code>__builtin_cpu_is("westmere")</code> returns a positive integer if
+      the run-time CPU is an Intel Core i7 Westmere processor.  Please refer
+      to the documentation for the list of valid CPU names recognized.</li>
+      <li>A built-in function <code>__builtin_cpu_supports</code> has been
+      added to detect if the run-time CPU supports a particular ISA feature.
+      It returns a positive integer on a match and zero otherwise.  It accepts
+      one string literal argument, the ISA feature.  For example,
+      <code>__builtin_cpu_supports("ssse3")</code> returns a positive integer
+      if the run-time CPU supports SSSE3 instructions.  Please refer to the
+      documentation for the list of valid ISA names recognized.</li>
+    </ul>
+    <p>Caveat: If these built-in functions are called before any static
+    constructors are invoked, like during IFUNC initialization, then the CPU
+    detection initialization must be explicity run using this newly provided
+    built-in function,  <code>__builtin_cpu_init</code>.  The initialization
+    needs to be done only once.  For example, this is how the invocation would
+    look like inside an IFUNC initializer:</p>
+    <pre>
+    static void (*some_ifunc_resolver(void))(void)
+    {
+      __builtin_cpu_init();
+      if (__builtin_cpu_is("amdfam10h") ...
+      if (__builtin_cpu_supports("popcnt") ...
+    }
+    </pre>
+    </li>
   </ul>
 
 <h3 id="mips">MIPS</h3>

Reply via email to