Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

__attribute__((aligned(x))) is sometimes ignored, and compiler misses alignment optimization opportunities. #2378

Closed
juj opened this issue May 23, 2014 · 4 comments

Comments

@juj
Copy link
Collaborator

juj commented May 23, 2014

If I write

struct __attribute__((packed)) foo
{
    int x;
    double d;
};

the compiler correctly generates a 12-byte structure, and any loads or stores of the int or the double are performed as unaligned U8 accesses. That's great, since I expect that 'packed' implies u8-alignment for all instances of that struct

If I write

// A
struct __attribute__((packed)) __attribute__((aligned(4))) foo
{
    int x;
    double d;
};

or

// B
struct __attribute__((packed)) foo
{
    int x;
    double d;
};
__attribute__((aligned(4))) foo *f = ...;
f->x = ...;

I expect that accesses to x and d would both occur as 4-byte loads and stores, in the first case all foos should be 4-byte aligned, and in the second case, accesses via that particular pointer should be 4-byte aligned. But it seems that in neither case they are not, and U8 accesses are generated. In particular, the following don't quite work either:

// C
__attribute__((aligned(1))) double *d = ...;
*d = ...; // Will perform a u64-aligned store and crash if d was unaligned.
__attribute__((aligned(4))) double *d = ...;
*d = ...; // same

which would be an amazing help when porting code, since it would allow typedeffing pointer types to unaligned data and perform selective conversion of code on a case-by-case basis.

If I write

// D
struct __attribute__((packed)) foo
{
    __attribute__((aligned(4))) int x;
    double d;
};

then accesses to x will come out as u32 loads and stores, which is great! But accesses to the double will not, and they are still referred to via u8 loads and stores :/ Here, the compiler should have been able to deduce that the double is u32-aligned as well and generate slightly more efficient code.

Writing

struct __attribute__((packed)) foo
{
    __attribute__((aligned(4))) int x;
    __attribute__((aligned(4))) double d;
};

will make accesses to both x and d to be u32 loads and stores, what one would expect in case. Can we fix cases A, B, C and D?

A test case is available at http://pastebin.com/ky2XY0wf . cc @sunfishcode , @kripken

@juj
Copy link
Collaborator Author

juj commented May 23, 2014

Playing around with the case C above, I notice that Emscripten/Clang does not recognize the aliased() attribute unless one explicitly creates a typedef for that. I.e. http://pastebin.com/sfwu9Bhh works and generates a four-byte memory access. Is that a LLVM bug?

@juj
Copy link
Collaborator Author

juj commented May 28, 2014

kripken added a commit that referenced this issue May 28, 2014
@kripken
Copy link
Member

kripken commented May 28, 2014

I added typedefs for this stuff in emscripten.h.

@stale
Copy link

stale bot commented Aug 31, 2019

This issue has been automatically marked as stale because there has been no activity in the past 2 years. It will be closed automatically if no further activity occurs in the next 7 days. Feel free to re-open at any time if this issue is still relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants