Re: PATCH: Add AVX support to x86-64 _dl_runtime_profile

[Ulrich Drepper <drepper at redhat dot com>]

H.J. Lu wrote:
> This is the updated patch against today's git. I fixed some white spaces
> and added vector register value check for audit_test.

I look at this and there are several problems.  Fixing it is not that
trivial time-wise so I leave it up to you.  The main problem is that
you're breaking the ABI.  The data structures in <bits/link.h> are used
by existing audit modules.  Existing code must continue to work.

Therefore the additional of the ymm registers must not disturb them.
The additional space must be allocated at the end (and no strong
alignment really, if it causes problems in the data structures).

This means that the low part of xmm0-7 will be represented twice.  This
introduces another problem but it is unavoidable.  We therefore always
have to store xmm0-7 and then the ymm register content conditionally
when the feature is available.  And the availability shouldn't be tested
at every use but instead only once.  Ideally at program startup time
when auditing is used.

The big problem introduced this way is how to handle the restoring of
the register contents.  Here it is clear that if the audit function
modifies the xmm[x] and ymm[x] value the code is wrong.  So we can
ignore this case.  Therefore I suggest we restore the ymm[x] content and
then check whether the xmm[x] content changed by comparing the exposed
value from the La_x86_64_regs and La_x86_64_retval data structures with
a shadow copy we made before the call to the audit function.  If it
changed we replace to the low half of the ymm register.

One final problem: your patch only handles 8 ymm registers?  Why?  Is
the calling convention so screwed up that it doesn't allow using all 16
ymm registers?

Please fix these issues and run the resulting code through the simulator
again.

-- 
â Ulrich Drepper â Red Hat, Inc. â 444 Castro St â Mountain View, CA â