|
Home > Archive > Unix Programming > October 2006 > "cast increases required alignment" SPARC ONLY
You are viewing an archived Text-only version of the thread.
To view this thread in it's original format and/or if you want to reply to
this thread please [click here]
| Author |
"cast increases required alignment" SPARC ONLY
|
|
| Frank Cusack 2006-10-19, 7:23 pm |
| $ gcc -O2 -g -D_REENTRANT -DUSE_SOCKET -DHAVE_PSET_INFO -DHAVE_CLOSEFROM -Wall -Wshadow -Wcast-align -Wsign-compare -Werror -c -o proc_thread.o proc_thread.c
proc_thread.c: In function `proc_getput':
proc_thread.c:109: warning: cast increases required alignment of target type
{
uint32_t *up;
up = (uint32_t *) &work->u.msg[12];
xmit->seq = *up;
}
xmit->seq is uint32, work->msg is uchar *
typedef struct workq_t {
union {
unsigned char msg[GSM_ULTRA_MSG_LEN];
uint32_t u; /* force uint32 aligned msg */
} u;
...
} workq_t;
typedef struct xmitq_t {
pthread_mutex_t mutex;
uint32_t seq; /* helix seqno */
...
} xmitq_t;
I want the -Wcast-align for other code (saved me in 1 case).
How do I avoid this problem? The extra 'up' var and tmp assignment
wasn't in the original code, I added that in a futile attempt to
work around this issue. Originally it was a direct assignment
xmit->seq = *((uint32_t *) &work->u.msg[12]);
I guess I can use memcpy() but it seems like a lot of work for 4 bytes
when assignment should work fine.
Why does it only show up on SPARC (NOTE: 32-bit)?
I'm using the same gcc version on both x86 and sparc (both solaris).
Does gcc know that x86 can handle misaligned reads? Still, -Wcast-align
should throw a warning, yes?.
thanks
-frank
| |
| Måns Rullgård 2006-10-19, 7:23 pm |
| Frank Cusack <fcusack@fcusack.com> writes:
> $ gcc -O2 -g -D_REENTRANT -DUSE_SOCKET -DHAVE_PSET_INFO -DHAVE_CLOSEFROM -Wall -Wshadow -Wcast-align -Wsign-compare -Werror -c -o proc_thread.o proc_thread.c
> proc_thread.c: In function `proc_getput':
> proc_thread.c:109: warning: cast increases required alignment of target type
[...]
> I want the -Wcast-align for other code (saved me in 1 case).
>
> How do I avoid this problem? The extra 'up' var and tmp assignment
> wasn't in the original code, I added that in a futile attempt to
> work around this issue. Originally it was a direct assignment
> xmit->seq = *((uint32_t *) &work->u.msg[12]);
> I guess I can use memcpy() but it seems like a lot of work for 4 bytes
> when assignment should work fine.
Casting to void *, then to uint32_t * should get rid of the warning.
> Why does it only show up on SPARC (NOTE: 32-bit)?
> I'm using the same gcc version on both x86 and sparc (both solaris).
> Does gcc know that x86 can handle misaligned reads?
Yes.
> Still, -Wcast-align should throw a warning, yes?.
Apparently not.
--
Måns Rullgård
mru@inprovide.com
| |
| noident@my-deja.com 2006-10-19, 7:23 pm |
| Frank Cusack wrote:
> Why does it only show up on SPARC (NOTE: 32-bit)?
> I'm using the same gcc version on both x86 and sparc (both solaris).
> Does gcc know that x86 can handle misaligned reads? Still, -Wcast-align
> should throw a warning, yes?.
I don't know for sure, but I'd say that it's common knowledge that
stuff like *((uint32_t *) &work->u.msg[any offset]) would work on x86,
so there is no requirement to align on a 4-byte boundary, hence no
warning.
In sparc, a code like that would compile on gcc but segfault if the
cast value does not happen to be properly aligned. Gcc sees that you
cast a uchar* to uint32_t* and gives you a warning (it's probably not
detecting that you take pains to make sure the cast value is aligned
properly).
So, basically, I think you're correct in your conjectures.
| |
| Frank Cusack 2006-10-20, 1:27 am |
| On Fri, 20 Oct 2006 01:01:26 +0100 Måns Rullgård <mru@inprovide.com> wrote:
> Casting to void *, then to uint32_t * should get rid of the warning.
That did the trick, thanks.
| |
| Maxim Yegorushkin 2006-10-20, 7:23 pm |
|
Frank Cusack wrote:
> $ gcc -O2 -g -D_REENTRANT -DUSE_SOCKET -DHAVE_PSET_INFO -DHAVE_CLOSEFROM -Wall -Wshadow -Wcast-align -Wsign-compare -Werror -c -o proc_thread.o proc_thread.c
> proc_thread.c: In function `proc_getput':
> proc_thread.c:109: warning: cast increases required alignment of target type
>
> {
> uint32_t *up;
>
> up = (uint32_t *) &work->u.msg[12];
> xmit->seq = *up;
> }
>
> xmit->seq is uint32, work->msg is uchar *
>
> typedef struct workq_t {
> union {
> unsigned char msg[GSM_ULTRA_MSG_LEN];
> uint32_t u; /* force uint32 aligned msg */
> } u;
> ...
> } workq_t;
Why not try something like this:
typedef struct workq_t {
union {
unsigned char msg[GSM_ULTRA_MSG_LEN];
uint32_t u[GSM_ULTRA_MSG_LEN / sizeof(uint32_t)];
/* force uint32 aligned msg */
} u;
...
} workq_t;
and then:
xmit->seq = work->u.msg[3];
?
| |
| Ulrich Eckhardt 2006-10-20, 7:23 pm |
| Frank Cusack wrote:
> proc_thread.c: In function `proc_getput':
> proc_thread.c:109: warning: cast increases required alignment
> of target type
>
> {
> uint32_t *up;
>
> up = (uint32_t *) &work->u.msg[12];
> xmit->seq = *up;
> }
>
> xmit->seq is uint32, work->msg is uchar *
Just so people don't get wrong assumptions, i.e. that this is just a
warning that you need to make go away somehow, there is one way to read a
(possibly misaligned) value correctly without causing bus errors and that
is memcpy(). If that is too much overhead is best answered by a profiler,
otherwise it is always a correct way and preferred until proven otherwise,
IMHO.
Further, there is even a way to read a value independent of endianess,
which is a must-do when reading files or parsing network traffic:
x = buf[0]+0x100*buf[1]+0x10000*buf[2]+0x1000000*buf[3];
Only fix the warning (others pointed out how) if you can guarantee that the
buffer is properly aligned.
Uli
--
http://www.erlenstar.demon.co.uk/unix/
| |
| Frank Cusack 2006-10-21, 1:27 am |
| On 20 Oct 2006 11:43:59 -0700 "Maxim Yegorushkin" <maxim.yegorushkin@gmail.com> wrote:
> Frank Cusack wrote:
>
> Why not try something like this:
>
> typedef struct workq_t {
> union {
> unsigned char msg[GSM_ULTRA_MSG_LEN];
> uint32_t u[GSM_ULTRA_MSG_LEN / sizeof(uint32_t)];
> /* force uint32 aligned msg */
> } u;
> ...
> } workq_t;
>
> and then:
>
> xmit->seq = work->u.msg[3];
You mean u.u[3] here.
Because it is illegal (undefined behavior) to read from a union
component other than the one you wrote to. Now, I know it will work
(each component is guaranteed to *start* at the same point within the
union), but I'd rather stick to portable constructs.
The void * trick works, is portable, and should generate the same
code. It's also more clear what part of msg I'm accessing when I use
u.msg[12].
Nice trick though. The real reason I didn't do that is I didn't think
of it. :-)
-frank
| |
| Maxim Yegorushkin 2006-10-21, 7:25 am |
|
Frank Cusack wrote:
> On 20 Oct 2006 11:43:59 -0700 "Maxim Yegorushkin" <maxim.yegorushkin@gmail.com> wrote:
>
> You mean u.u[3] here.
>
> Because it is illegal (undefined behavior) to read from a union
> component other than the one you wrote to. Now, I know it will work
> (each component is guaranteed to *start* at the same point within the
> union), but I'd rather stick to portable constructs.
>
> The void * trick works, is portable, and should generate the same
> code. It's also more clear what part of msg I'm accessing when I use
> u.msg[12].
There is no difference between these two tricks. Casting to void* does
not make it portable in any way. A certain byte-sex is assumed, this
what makes it nonportable in the first place.
One portable way is:
uint32_t = work->msg[12] | work->msg[13] << 8 | work->msg[14] << 16
| work->msg[15] << 24;
| |
| Frank Cusack 2006-10-21, 1:31 pm |
| On 21 Oct 2006 03:31:51 -0700 "Maxim Yegorushkin" <maxim.yegorushkin@gmail.com> wrote:
> Frank Cusack wrote:
>
> There is no difference between these two tricks. Casting to void* does
> not make it portable in any way. A certain byte-sex is assumed, this
> what makes it nonportable in the first place.
Without seeing the rest of the code, you can't say that. I might store
the data in u.msg in the right order to begin with.
In this case, I don't; the data is stored little-endian even on
big-endian machines and when I save the uchar data as uint32, I get a
different value (in the uint32 than was written into the uchar array)
on big-endian machines. But it is still portable. The fact that I
get the "wrong" value on a big-endian machine doesn't make it
non-portable, it just makes it semantically "incorrect". Quotes used
liberally because I do this intentionally and it doesn't matter what
the actual value is for my application.
Even in the general case, pulling out each byte and multiplying or
shifting doesn't guarantee correctness; you have to assume a certain
endianness that the data was stored as.
-frank
| |
| Rainer Weikusat 2006-10-23, 7:16 am |
| Ulrich Eckhardt <doomster@knuut.de> writes:
[...]
> Further, there is even a way to read a value independent of endianess,
> which is a must-do when reading files or parsing network traffic:
>
> x = buf[0]+0x100*buf[1]+0x10000*buf[2]+0x1000000*buf[3];
This is semantically the same as
x = *buf | (buf[1] << 8) | (buf[2]) << 16) | (buf[3] << 24)
But I think the second is preferable, because it describes what is
actually being done (four octets in LSB-order are assembled into a
32-bit integer). Additionally, this is of course not endian-independant but only
host-endian indenpendant and the stored byte order you assume is
little-endian, ie not network byte order.
| |
| Frank Cusack 2006-10-23, 7:21 pm |
| On Mon, 23 Oct 2006 10:51:59 +0200 Rainer Weikusat <rainer.weikusat@sncag.com> wrote:
> Ulrich Eckhardt <doomster@knuut.de> writes:
>
> [...]
>
>
> This is semantically the same as
>
> x = *buf | (buf[1] << 8) | (buf[2]) << 16) | (buf[3] << 24)
>
> But I think the second is preferable, because it describes what is
> actually being done
and probably much faster.
-frank
| |
| Logan Shaw 2006-10-24, 1:23 am |
| Frank Cusack wrote:
> On Mon, 23 Oct 2006 10:51:59 +0200 Rainer Weikusat <rainer.weikusat@sncag.com> wrote:
>
> and probably much faster.
Possibly, but I don't know if I would go so far as to say probably.
It's not exactly a difficult optimization to turn "multiply by N"
into "shift left" when N is a power of two.
All you have to do is walk the abstract syntax tree, look for
multiplication operators that have one argument which is a power
of 2 (and there are a finite number of powers of two to worry
about for any halfway-normal target architecture, so you can
just hard code them if you want). When you find
*
/ \
N expr
or
*
/ \
expr N
and N is a power of 2 and a constant, you just replace it with
<<
/ \
expr log2(N)
You probably also want to make sure left-shift behaves right in the
case of signed integers (if you have those) and so on, but that's
the basic idea.
- Logan
| |
| Frank Cusack 2006-10-24, 7:21 am |
| On Tue, 24 Oct 2006 04:50:37 GMT Logan Shaw <lshaw-usenet@austin.rr.com> wrote:
> Frank Cusack wrote:
>
> Possibly, but I don't know if I would go so far as to say probably.
> It's not exactly a difficult optimization to turn "multiply by N"
> into "shift left" when N is a power of two.
Yup, and I would have thought this too except that in my experience a
simple %8 operation which should be optimized into a mask, is not
(gcc -O2). A quick test shows that even without optimization (gcc -O0)
the multiplies are turned into shifts.
-frank
| |
| Alex Fraser 2006-10-24, 1:17 pm |
| "Frank Cusack" <fcusack@fcusack.com> wrote in message
news:m2fyde2kmu.fsf@sucksless.local...
> On Tue, 24 Oct 2006 04:50:37 GMT Logan Shaw <lshaw-usenet@austin.rr.com>
> wrote:
>
> Yup, and I would have thought this too except that in my experience a
> simple %8 operation which should be optimized into a mask, is not
> (gcc -O2). A quick test shows that even without optimization (gcc -O0)
> the multiplies are turned into shifts.
I think a "simple %8 operation" is not as simple as you think ;). Try with
left operands of an unsigned type and you will probably find division and
modulus by a constant power of two are converted to a right shift and
bitwise AND respectively.
Alex
| |
| Frank Cusack 2006-10-24, 1:17 pm |
| On Tue, 24 Oct 2006 17:34:22 +0100 "Alex Fraser" <me@privacy.net> wrote:
> "Frank Cusack" <fcusack@fcusack.com> wrote in message
> news:m2fyde2kmu.fsf@sucksless.local...
>
> I think a "simple %8 operation" is not as simple as you think ;). Try with
> left operands of an unsigned type and you will probably find division and
> modulus by a constant power of two are converted to a right shift and
> bitwise AND respectively.
hmm maybe I was using signed types, but I found that obvious optimization
to not be the case.
| |
| Eric Sosman 2006-10-24, 1:17 pm |
|
Alex Fraser wrote On 10/24/06 12:34,:
> "Frank Cusack" <fcusack@fcusack.com> wrote in message
> news:m2fyde2kmu.fsf@sucksless.local...
>
>
>
> I think a "simple %8 operation" is not as simple as you think ;). Try with
> left operands of an unsigned type and you will probably find division and
> modulus by a constant power of two are converted to a right shift and
> bitwise AND respectively.
... but make sure the unsigned type is at least as wide
as `unsigned int'. Anything narrower will be converted to
plain `signed int', even if it was unsigned to begin with:
unsigned char x = 42;
unsigned char y = x % 8; // signed[*]
y = (unsigned short)x % 8; // signed[*]
y = (unsigned int)x % 8; // unsigned (whew!)
[*] Assuming char and short are narrower than int.
--
Eric.Sosman@sun.com
| |
| Alex Fraser 2006-10-24, 7:17 pm |
| "Frank Cusack" <fcusack@fcusack.com> wrote in message
news:m2d58hbqo5.fsf@sucksless.local...
> On Tue, 24 Oct 2006 17:34:22 +0100 "Alex Fraser" <me@privacy.net> wrote:
>
> hmm maybe I was using signed types, but I found that obvious optimization
> to not be the case.
Exactly; I don't think you can optimise eg "% 8" to a mask if the left
operand has a signed type, because the result would be incorrect[1] if the
left operand was negative.
[1] That is, inconsistent with integer division rounding towards zero, which
is typically what you get. (Maybe as required in C99 - open to correction on
this.)
Alex
| |
| Alex Fraser 2006-10-24, 7:17 pm |
| "Eric Sosman" <Eric.Sosman@sun.com> wrote in message
news:1161709022.983433@news1nwk...
> Alex Fraser wrote On 10/24/06 12:34,:
>
> ... but make sure the unsigned type is at least as wide
> as `unsigned int'. Anything narrower will be converted to
> plain `signed int', even if it was unsigned to begin with:
Good point, but it is probably irrelevant in practice. I think the key is
the possibility that the (promoted) value is negative; if (and only if) it
is, a shift/bitwise AND will give the wrong result, but the value resulting
from integer promotion of a "small" unsigned type can never be negative.
Alex
|
|
|
|
|