[PATCH] checked in prefetch support for mips

[Eric Christopher]

On Wed, 2002-06-19 at 16:45, cgd@broadcom.com wrote:
> A few notes, of course.  8-)
> 

Of course, I can't check in patches without at least some comment ;)


> * Err, uh, even with this "non-simple" algorithm, I think you're
>   getting the hints wrong.  8-)
> 

In what way?

>   locality seems to go from 0 (not much locality) to 3 (high degree of
>   locality), right?
> 
>   So, in terms of the pref codes:
> 
> 	0/1 are "normal"
> 	4/5 are "streamed" (low locality)
> 	6/7 are "retainted" (high locality)
> 

Yep.

> * I suspect that people will want to pick which pref ops to used based
>   on ... i dunno whether it would be arch or tune.  (i think there's
>   an argument for both. 8-)  However, I don't really want to go there
>   yet.  8-)
> 

Then we'll ignore it.

> * Were the prefetch ops and hints well-defined "architecturally" for
>   MIPS IV (don't think so), or did implementations just happen to
>   choose a (somewhat-)consistent set of hints?  (and, did any
>   implementations just punt the opcodes?)
> 

I looked at the mips32 and 64 docs since those were what I had handy.
I've not found an example otherwise.

> * you never use use prefx; is that just because there's not enough
>   flexibility in the prefetch RTL node to allow it, or ???
> 

Correct. I don't believe indexed addressing works for this.

> * lastly, it might be nice to have MD builtins that let code generate
>   pref/prefx w/ arbitrary hints...  that way people could code the
>   'exact' prefetches that they want, and could also generate nudge or
>   prepare_for_store in a scheduler-friendly way...
> 

You should read the prefetch examples. They are most enlightening. :)

> 
> Based on the above, I think you want something like the UNTESTED diff
> below to your current sources.  8-)
> 
> And yes, I know i'm welcome to submit tested patches that implement
> the "desirable" new features listed above.  8-)
> 
>      static const char * const alt[2][4] = {
>  	{
> -	 "pref\t0,%a0",
> -	 "pref\t4,%a0",
>  	 "pref\t4,%a0",
> +	 "pref\t0,%a0",
> +	 "pref\t0,%a0",
>  	 "pref\t6,%a0"
>  	},
>  	{
> -	 "pref\t1,%a0",
> -	 "pref\t5,%a0",
>  	 "pref\t5,%a0",
> +	 "pref\t1,%a0",
> +	 "pref\t1,%a0",
>  	 "pref\t7,%a0"

Umm.. Why these changes? Can you explain why? You didn't above.

> -/* ISA has data prefetch instruction.  */
> -#define ISA_HAS_PREFETCH	((ISA_MIPS4				\
> -				  || ISA_MIPS32				\
> +/* ISA has data prefetch instruction.  (MIPS IV and MIPS V have prefetch
> +   instructions, but they are not architecturally standard.  Add checks
> +   for specific CPUs to this list and to mips_emit_prefetch() as needed.  */
> +#define ISA_HAS_PREFETCH	((ISA_MIPS32				\
>  				  || ISA_MIPS64)	       		\

The comment part is ok, I don't like removing mips4 because it _did_
have a prefetch instruction. I'll assume, that since the docs I have
mention mips4 and don't mention any functional changes that the current
support should be at least sufficient.

-eric

-- 
I will not carve gods