Skip to content
This repository has been archived by the owner on Sep 27, 2019. It is now read-only.

Array allocation of TupleHeader fails compilation on Ubuntu 18.04 #1433

Open
pmenon opened this issue Jun 26, 2018 · 3 comments
Open

Array allocation of TupleHeader fails compilation on Ubuntu 18.04 #1433

pmenon opened this issue Jun 26, 2018 · 3 comments
Assignees
Labels

Comments

@pmenon
Copy link
Member

pmenon commented Jun 26, 2018

I just tried to compile latest master on Ubuntu 18.04 GCC7, and it looks to be failing compilation when instantiating the array of TupleHeaders in the TileGroup here. The problem stems from the fact that TupleHeader is marked as 64-byte aligned (i.e., __attribute__((aligned(64)))) , but array allocations don't accept alignment parameters and, hence, don't guarantee alignment.

We can add the compiler flag -faligned-new to handle it - not sure how it plays with GCC5/6. We also don't support Ubuntu 18.04 officially, but wanted to track it when we do start.

@mbutrovich
Copy link
Contributor

I suspect that the answer is no since it's probably the same underlying implementation, but does it work with struct alignas(64) TupleHeader on the type definition instead of __attribute__((aligned(64)))?

@mbutrovich
Copy link
Contributor

mbutrovich commented Jun 26, 2018

Nope, doesn't seem like that'll change anything. In the end it might just be a premature optimization that would be safe to remove. Sorry for causing trouble on 18.04/GCC7.

I was trying to get each tuple's header to be cache line aligned, but perhaps it's too early to worry about that. Interestingly if I take out the attribute, I always end up with a 64-byte aligned base address for the array anyway, but that's probably just getting lucky.

@pmenon
Copy link
Member Author

pmenon commented Jun 26, 2018

@mbutrovich You're just getting lucky, the standard makes no guarantees on alignment.

I understand the motivation, it's probably is okay to keep it. If you really need alignment, over-allocate and use std::align. We do this for parallel execution when allocating per-thread states.

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

No branches or pull requests

2 participants