Re: POINTER_ALIGNED_P (was: Re: CVS commit: src/sys)

[Valery Ushakov <uwe%stderr.spb.ru@localhost>]

On Tue, Feb 16, 2021 at 22:56:19 +0100, Roland Illig wrote:

> On 16.02.2021 19:46, Roy Marples wrote:
> > I suggest we change POINTER_ALIGNED_P to accept the alignment value we
> > want rather than the bitwise test we supply, like so:
> > 
> > #define POINTER_ALIGNED_P(p, a) (((uintptr_t)(p) & ((a) - 1)))
> 
> That's a good definition, clear, simple, obvious, without any surprises.

Now, replace the value "a" with the type "t" and change (a) to
sizeof(t) and you will get ALIGNED_POINTER from just above.


> To make sure that this macro doesn't get broken again, I have written
> the attached tests.  I will commit them after making sure I got the
> changes to distrib/sets/lists/tests/mi correct.  All the rest I already
> tested successfully.

I don't see any proposal to change the meaning of this macro to
actually require the alignment even for arches without strict
alignment.  Does the attached test really passes on e.g. x86 where the
macro is always true?  E.g. this one:

> +	if (POINTER_ALIGNED_P(p + 1, 2))
> +		atf_tc_fail("p + 1 must not be aligned to 2");


> Is my assumption correct that on each platform supported by NetBSD, a
> variable of type double gets aligned to a multiple of 8, even on the
> stack?

No.  E.g. on sh3 doubles are 8 bytes but are 4 bytes aligned.  I'm
almost sure some other ABI has that kind less strict alignment too,
but I don't remember.


> I wanted to keep the test as simple as possible, therefore I
> didn't want to call malloc just to get an aligned pointer.

You can use an ordinary array that is large enough and manually
find/construct a suitably aligned pointer value inside that array.


BUT, can we, PLEASE, stop making random breaking changes and at least
articulate first what is that that we want here?

POINTER_ALIGNED_P should have never been brought into existence in the
first place.

ALIGNED_POINTER seems to be used to define BUS_SPACE_ALIGNED_POINTER -
the real fun here is sys/arch/x86/include/bus_defs.h that defines
BUS_SPACE_ALIGNED_POINTER to be "really" aligned for BUS_SPACE_DEBUG,
which seems kinda suspicious to me.  BTW, can we really conclude from
the CPU's alignment requirements to some bus alignment requirements?

But to get back to my main point, PLEASE, can we stop making random
aimless changes without prior discussion?

To quote Vonnegut, "If I had of been a observer, I would of said we
was comical."


-uwe