On Tue, 28 Jan 2003 13:10:41 -0800
Ian Romanick <[EMAIL PROTECTED]> wrote:

> Felix Kühling wrote:
> > It seems I like answering my own mails ;-)
> > 
> > I fixed this problem but it is probably not optimal. A simple patch is
> > attached. It seems that this error was introduced by an atempt to
> > optimize prefetching. The next normal was read into MM0 and MM1 at the
> > end of the G3TN_norm loop. So in the first cycle MM0 and MM1 were
> > undefined. Furthermore it read one extra normal at the end of the array
> > which was never processed.
> > 
> > The patch moves the load operations back to the front of the loop as in
> > the G3TN_norm_w_lengths case.
> 
> Good catch.  It looks like this went into the Mesa tree back in October 
> of 2001...over a year ago!  It looks like Andres Lewycky gave Brian some 
> bad patches. :(

Yeah, but until November 2002 (DRI trunk) there was a comment in 3dnow.c
that the 3dnow-normal code is broken and it was not used.

> I realize that AMD recommends reading memory backwards, but would a 
> quick-fix be to just use the 3Dnow! prefetch instructions?

The prefetch instructions used are and must be 3DNow instructions. On
Intel Prefetch was introduced with the SSE extension on the PentiumIII.
They're not available on older Athlons and K6's. Anyway, all that
prefetching looks odd to me. In the first transform loop in
_mesa_3dnow_transform_normalize_normals memory is prefetched which is
never read but only written. This is obviously useless. Then in the
normalize loop the memory which was written before is prefetched again.
I think this is not necessary. The array is small enough to be still in
the cache.

I'll see if I can clean this up a bit. On the mesa-4-0-4 branch this
code is disabled anyway, so there is not really a hurry to apply my
stupid little patch. About this reading backward thing, where is that
documented. I have an AMD Athlon optimization guide from February 2002
which doesn't mention it.

> Since these functions are globally exported, it might be worth it to 
> write a quick test that calls the various _transform_normalize_normals 
> functions to make sure that they all produces the same (or close enough) 
> results.

And:
_transform_normalize_normals_no_rot
_transform_rescale_normals_no_rot
_transform_rescale_normals
_transform_normals_no_rot
_transform_normals
_normalize_normals
_rescale_normals

These should be tested too, while we're at it.

Felix

               __\|/__    ___     ___     ___
__Tschüß_______\_6 6_/___/__ \___/__ \___/___\___You can do anything,___
_____Felix_______\Ä/\ \_____\ \_____\ \______U___just not everything____
  [EMAIL PROTECTED]    >o<__/   \___/   \___/        at the same time!


-------------------------------------------------------
This SF.NET email is sponsored by:
SourceForge Enterprise Edition + IBM + LinuxWorld = Something 2 See!
http://www.vasoftware.com
_______________________________________________
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel

Reply via email to