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

Support gcc7 #70

Merged
merged 1 commit into from
Feb 2, 2017
Merged

Support gcc7 #70

merged 1 commit into from
Feb 2, 2017

Conversation

herumi
Copy link
Contributor

@herumi herumi commented Feb 1, 2017

gcc 7 put the following message, then I add -faligned-new option, but I don't know that it is best way.

src/counter/handler.cpp: In member function 'std::unique_ptr<cybozu::tcp_socket> yrmcds::counter::handler::make_counter_socket(int)':
src/counter/handler.cpp:72:47: warning: 'new' of type 'yrmcds::counter::counter_socket' with extended alignment 64 [-Waligned-new=]
         new counter_socket(s, m_finder, m_hash) );

@nojima
Copy link
Collaborator

nojima commented Feb 1, 2017

I'll take a look later.

@nojima
Copy link
Collaborator

nojima commented Feb 1, 2017

I've got what the warning says.

When allocating an object whose alignment is larger than alignof(max_align_t) by using new-expression, the result is implementation-defined.

GCC 6 seems to ignore alignas(64), and allocates the object aligned as 16 byes.
I confirmed that GCC 7 respects alignas(64) when -faligned-new is specified.

@nojima
Copy link
Collaborator

nojima commented Feb 1, 2017

Rather than -faligned-new, I would like to remove alignas(CACHE_LINE_SIZE) from yrmcds::counter::counter_socket.
We can safely remove it because it is virtually not working until now.

It is not too late to use alignas after contentions among threads get to be real problem.

@ymmt2005
Copy link
Member

ymmt2005 commented Feb 1, 2017

@nojima Agreed. Thank you for your investigation.

@ymmt2005
Copy link
Member

ymmt2005 commented Feb 2, 2017

@herumi
Could you please drop commit 50a5092 from this PR?
I will take over the alignment problems in another commit.

@ymmt2005 ymmt2005 merged commit a7026e0 into cybozu:master Feb 2, 2017
@ymmt2005
Copy link
Member

ymmt2005 commented Feb 2, 2017

merged. Thank you!

@ymmt2005
Copy link
Member

ymmt2005 commented Feb 2, 2017

The alignment problem will be addressed in #71. @nojima

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

Successfully merging this pull request may close these issues.

3 participants