On 8/13/24 11:18, Andrew Carlotti wrote:
I'm still waiting for review for this patch. I've asked Richard
Sandiford about it, and he'd like a docs maintainer to review the
patch (so I've cc'd the rest of them now as well).
I'm sorry, I've seen this patch go by before, but every time I've looked
at it I get confused by which patch hunk belongs to which section, and
tracking which are just moving text around and which are new, and I
haven't had the time to build the manual with the patch and compare the
changed sections. So I can't comment right now on whether the
organization changes are OK.
Not being familiar with the functionality, I don't think I have anything
intelligent to say about it from a technical perspective, either. Has
someone else reviewed this for correctness already?
I did look it over again for other problems, and this is what jumped out
at me.
The reference to a requirement for ELF and GLIBC 2.23 buried in an
implementation details discussion concerns me. If this feature doesn't
work on non-ELF or non-GLIBC targets (Windows, bare-metal, Android...)
that's not an implementation detail, it's user-visible restriction and
needs to be documented up front.
Please get rid of all the future tense ("will") and write in the present
tense, assuming all the documented functionality is presently
implemented. :-)
There are multiple places where you need to do
s/at runtime/at run time/g
per https://gcc.gnu.org/codingconventions.html#Spelling
Also please
s/target specific version/target-specific version/
In this paragraph:
+In the above example, four versions of function foo are created. The first
+version of @samp{foo}, with the target attribute @samp{"default"}, is the
+default version. This version gets executed when no other target specific
+version qualifies for execution on a particular platform. Other versions of
+@samp{foo} are created by using the same function signature but with a
+different target string. The function @samp{foo} can be called or a pointer to
+it can be taken just like for a regular function. GCC takes care of doing the
+dispatching to call the right version at runtime.
I believe all of these @samp{} things should be @code{}, and you missed
adding markup to the first instance of "foo".
And here
+Function multiversioning is implemented using the STT_GNU_IFUNC symbol type
@code{STT_GNU_IFUNC}, please.
-Sandra
