Re: [PATCH] data prefetch support and __builtin_prefetch (take 2)

[Janis Johnson <janis187 at us dot ibm dot com>]

On Tue, Dec 04, 2001 at 02:27:04PM +0000, Richard Earnshaw wrote:
> 2001-12-03  Janis Johnson  <janis187@us.ibm.com>
> 
>         * rtl.def (PREFETCH): New rtx code.
>         * doc/rtl.texi (PREFETCH): Add documentation.
>         * function.c (instantiate_virtual_regs_1): Handle PREFETCH rtx.
>         * rtlanal.c (reg_referenced_p): Ditto.
>         * sched-vis.c (print_exp):  Ditto.
>         * ssa-dce.c (find_inherently_necessary):  Ditto.
[snip]
> 
>         * builtin-types.def (BT_FN_VOID_PTR_INT_INT): New.
>         * builtins.def (BUILT_IN_PREFETCH): New.
>         * builtins.c (expand_builtin_expect): New.
>           (expand_builtin): Call it.
>         * doc/extend.texi: Document __builtin_expect.
> 
> Argh!  This patch breaks the build on ARM platforms.  Three problems with 
> it:
> 
> 1) The arm.md already has a "prefetch" expander that takes a different 
> number of arguments.  (I can fix this, but it should have been fixed in 
> the above check-in).

Sorry, I should have done more searching for possible conflicts.  Let's
get the next piece figured out and then you can let me know whether you
want me to fix this or do it yourself.

> 2) ARM targets already have a __builtin_prefetch function that takes one 
> argument; as far as I can tell your new implementation will mean an API 
> change for users of gcc-3.0

I looked through all of the documentation to see if there was existing
prefetch support and all I found was __builtin_ia32_prefetch.  Now I'll
know to look through the target-dependent code, too, for undocumented
builtin functions.

I suppose it's too much to hope for that since it's not documented, no
one knows about it and it's not being used.

Assuming, though, that it is being used, do I need to change the name of
the new generic builtin to avoid conflicting, or is the use limited to a
small amount of code within ARM so the existing uses could be changed?
__builtin_prefetch is such an obvious choice for a name, but I can start
thinking about alternatives.
 
> 3) There's no documentation of the "prefetch" expander in md.texi

Thanks for catching that, I didn't know about documenting it there.
I'll submit it later today.

> and finally,
> 
> 4)      * builtins.c (expand_builtin_expect): New.
>           (expand_builtin): Call it.
>         * doc/extend.texi: Document __builtin_expect.
> 
> Don't you mean "prefetch" not "expect"?

Eek!  Yes, I'll fix this.

Janis