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.

Sign up for GitHub

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

Jump to bottom

GCC-12 fails to inline XXX_FORCED_INLINE functions, XXH3_accumulate_512_sse2(), and XXH3_scrambleAcc_sse2() with -Og flag. #800

Closed
zephyrus00jp opened this issue Feb 18, 2023 · 7 comments · May be fixed by #802
Closed

GCC-12 fails to inline XXX_FORCED_INLINE functions, XXH3_accumulate_512_sse2(), and XXH3_scrambleAcc_sse2() with -Og flag. #800

zephyrus00jp opened this issue Feb 18, 2023 · 7 comments · May be fixed by #802

Comments

@zephyrus00jp
Copy link

Hi, I tried compiling Thunderbird mail client using GCC-12.
Well, with -Og flag, it fails to inline the following two functions and compilation aborts.

XXH3_accumulate_512_sse2()
XXH3_scrambleAcc_sse2()

Without -Og, the compilation succeeds.
Mozilla bugzilla is here.
https://bugzilla.mozilla.org/show_bug.cgi?id=1817565

I was advised to post a bug report at this github repository.

I think we may need to enable/disable XXX_FORCED_INLINE attribute for these two functions depending on the GCC version and
whether -Og is used. But according to gcc bugzilla, there is currently no such macro that is defined when -Og is used on the command line.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107931

Come to think of it, macro is a preprocessor feature and so whether -Og is used or not is no concern to the preprocessor.
So it figures.

I need to settle on a certain macro such as __GCC_12_FAILS_ALWAYS_INLINE_WITH_DEBUG_OPT
and wrap the XXX_FORCED_INLINE attribute for the two functions as follows.
#if ! defined(__GCC_12_FAILS_ALWAYS_INLINE_WITH_DEBUG_OPT)
XXX_FORCED_INLINE
#endif

And then when I use -Og, I will define -D__GCC_12FAILS_ALWAYS_INLINE_WITH_DEBUG_OPT
and unless this macro is defined, all is well. The functions would be inlined. (At least GCC will not complain.)

It seems to be a compiler-specific and version-specific problem (GCC-10, GCC-11 did not report the errors. B now I am not quite convinced whether they really inlined the functions or not. According the gcc bugzilla, sometimes errors were/are not reported.)
So there is not much we can do. I just wanted to let you know the issue exists.

BUT if some measures are taken here, I don't have to modify my local Mozilla source code which imports xxhash source tree, and everybody who would compile xxHash with gcc-12 and -Og will be happier about it. :-)

Thank you for making a very fast hash library available to the general programming community.
@Cyan4973
Copy link
Owner

Indeed, there are known issues when compiling libxxhash with the -Og flag on modern gcc version : #720

Unfortunately, there is no "generic" solution yet,
partially because -Og doesn't provide any indication that it is being employed to the preprocessor, so it can't adjust the source code.

It's unclear if there will be a solution to this problem, or if it's something that can only be fixed on the compiler side.

A manual work around involves combining this flag with an appropriate xxhash build macro, such as XXH_NO_INLINE_HINTS, though it obviously impacts performance.

@easyaspi314
Copy link
Contributor

easyaspi314 commented Feb 19, 2023 •
edited
Loading

I believe the solution is to not mark the functions that are turned into pointers force inline, as -Og tries to guarantee that pointers are not optimized out.

@Cyan4973
Copy link
Owner

I believe the solution is to not mark the functions that are turned into pointers force inline, as -Og tries to guarantee that pointers are not optimized out.

Well, this is what XXH_NO_INLINE_HINTS achieves.
So there is a work around.
I presume the issue is that this build macro is not automatically triggered when compiling with -Og.
And it's unclear if this can be done, since -Og doesn't expose any standard macro for detection.

@zephyrus00jp
Copy link
Author

zephyrus00jp commented Feb 21, 2023 •
edited
Loading

Ｉ think I will define

__NO_INLINE__

manually when I compile thunderbird mail client with GCC-12 and -Og flag.
That seems to take care of the issue.
Indeed, I could compile the code with -Og using gcc-12 after -D__NO_INLINE__ is added.

#720 (comment)

@easyaspi314
Copy link
Contributor

easyaspi314 commented Feb 21, 2023 •
edited
Loading

Ｉ think I will define

__NO_INLINE__

manually when I compile thunderbird mail client with GCC-12 and -Og flag.
That seems to take care of the issue.
Indeed, I could compile the code with -Og using gcc-12 after -D__NO_INLINE__ is added.

#720 (comment)

-DXXH_NO_INLINE_HINTS=1 functionally does the same thing without modifying reserved macros. xxHash selects these things when these macros are defined but they all can be defined manually.

This was referenced Feb 23, 2023
Draft
Merged
@t-mat t-mat mentioned this issue Mar 3, 2023
Closed
@t-mat
Copy link
Contributor

t-mat commented Mar 12, 2023

I think #804 fixes this issue. If you can merge it, please let us know what happens in your project.

@thesamesam thesamesam mentioned this issue Apr 21, 2023
Open
@t-mat
Copy link
Contributor

t-mat commented Jul 1, 2023

We can close this issue, since #802, #804 and #810 have resolved/ensured it. Ping @Cyan4973

@Cyan4973 Cyan4973 closed this as completed Jul 1, 2023
@mattbenjamin mattbenjamin mentioned this issue Feb 15, 2024
Merged
14 tasks
@Cyan4973 Cyan4973 mentioned this issue Dec 8, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Refactor to improve code size, fix GCC 12 easyaspi314/xxHash
4 participants
@Cyan4973 @t-mat @easyaspi314 @zephyrus00jp