Skip to content

Add ALIGNED(x) support#2410

Open
jopadan wants to merge 41 commits into
fabiangreffrath:masterfrom
jopadan:master
Open

Add ALIGNED(x) support#2410
jopadan wants to merge 41 commits into
fabiangreffrath:masterfrom
jopadan:master

Conversation

@jopadan
Copy link
Copy Markdown

@jopadan jopadan commented Oct 4, 2025

Pack and Aligned some alignable types.
x must be a power of 2 integer constant.
Using expressions like sizeof(float)*2 won't work

Jon Daniel and others added 30 commits November 23, 2024 08:25
@fabiangreffrath
Copy link
Copy Markdown
Owner

Is there any immediate advantage in doing this?

Comment thread src/doomdata.h
Comment thread src/doomtype.h Outdated
Comment thread src/m_vector.h
typedef float quat __attribute__((ext_vector_type(4)));
#else
typedef struct
#pragma pack(push, 1)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you only using the #pragma pack from MSVC and not the PACKED_PREFIX and PACKED_SUFFIX macros?

What is the benefit of setting the alignment here?

Copy link
Copy Markdown
Author

@jopadan jopadan Oct 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The #pragma pack(push,1)/#pragma pack(pop) works for all supported compilers. Not all the MSVC compilers support __attribute__ directive used in PACKED_PREFIX.
Probably the combination of PACKED_PREFIX and ALIGNED won't work.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, but why do we need ALIGNED? I suppose it could theoretically improve performance if the compiler uses SIMD vector instructions. Did you measure the improvement?
For now, it seems like an unnecessary complication that might become obsolete with future CPUs or compilers.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jopadan could you please elaborate on the "why should we?" question?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants