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

media-libs/x264-9999: endian test failed #671

Closed
ghost opened this issue Dec 20, 2020 · 10 comments
Closed

media-libs/x264-9999: endian test failed #671

ghost opened this issue Dec 20, 2020 · 10 comments

Comments

@ghost
Copy link

ghost commented Dec 20, 2020

In this commit "-c" is (re-)intoduced to the conftest compilation. This breaks the endian test for my environment. Dropping "-c" again fixes the issue:

x264-fix-endian-test.zip
or
#672

"-c" jas been introduced here: e96d39e

The same for x264-encoder - patch updated.

@ghost
Copy link
Author

ghost commented Dec 20, 2020

Just a quick test:

adler tmp # echo "int i[2] = {0x42494745,0}; double f[2] = {0x1.0656e6469616ep+102,0};" > conftest.c
adler tmp # x86_64-pc-linux-gnu-gcc conftest.c -march=bdver1 -mtune=bdver1 -O3 -fgraphite-identity -floop-nest-optimize -fdevirtualize-at-ltrans -fipa-pta -fno-semantic-interposition -flto=8 -fuse-linker-plugin -pipe -Wl,-O1 -Wl,--as-needed -Wl,--hash-style=gnu -Wall -I. -Wl,-O1 -Wl,--as-needed -Wl,--hash-style=gnu -march=bdver1 -c -shared -o conftest-with_-c.o
adler tmp # x86_64-pc-linux-gnu-gcc conftest.c -march=bdver1 -mtune=bdver1 -O3 -fgraphite-identity -floop-nest-optimize -fdevirtualize-at-ltrans -fipa-pta -fno-semantic-interposition -flto=8 -fuse-linker-plugin -pipe -Wl,-O1 -Wl,--as-needed -Wl,--hash-style=gnu -Wall -I. -Wl,-O1 -Wl,--as-needed -Wl,--hash-style=gnu -march=bdver1 -shared -o conftest-without_-c.o
adler tmp # strings conftest-with_-c.o |grep -e EGIB -e naidnePF -e BIGE -e FPendian
adler tmp # strings conftest-without_-c.o |grep -e EGIB -e naidnePF -e BIGE -e FPendian
EGIB
naidnePF
adler tmp #

@jiblime
Copy link
Contributor

jiblime commented Dec 23, 2020

I think it would be best to create a new shell script in bashrc.d/ and use regex to make sure it stays the same. Would it add noticeable overhead to the ebuild process to have the script check for the package being media-libs/x264 every time?

@ghost
Copy link
Author

ghost commented Dec 23, 2020

I don't even understand what the second "it" is refering to. As long as your script works, I'm the last insisting on removing -c from the patch.

@jiblime
Copy link
Contributor

jiblime commented Dec 24, 2020

Second it is referring to adding an additional script to bashrc.d/. And yes, I agree your patch works. Please use code blocks when pasting output.

adler tmp # echo "int i[2] = {0x42494745,0}; double f[2] = {0x1.0656e6469616ep+102,0};" > conftest.c
adler tmp # x86_64-pc-linux-gnu-gcc conftest.c -march=bdver1 -mtune=bdver1 -O3 -fgraphite-identity -floop-nest-optimize -fdevirtualize-at-ltrans -fipa-pta -fno-semantic-interposition -flto=8 -fuse-linker-plugin -pipe -Wl,-O1 -Wl,--as-needed -Wl,--hash-style=gnu -Wall -I. -Wl,-O1 -Wl,--as-needed -Wl,--hash-style=gnu -march=bdver1 -c -shared -o conftest-with_-c.o
adler tmp # x86_64-pc-linux-gnu-gcc conftest.c -march=bdver1 -mtune=bdver1 -O3 -fgraphite-identity -floop-nest-optimize -fdevirtualize-at-ltrans -fipa-pta -fno-semantic-interposition -flto=8 -fuse-linker-plugin -pipe -Wl,-O1 -Wl,--as-needed -Wl,--hash-style=gnu -Wall -I. -Wl,-O1 -Wl,--as-needed -Wl,--hash-style=gnu -march=bdver1 -shared -o conftest-without_-c.o
adler tmp # strings conftest-with_-c.o |grep -e EGIB -e naidnePF -e BIGE -e FPendian
adler tmp # strings conftest-without_-c.o |grep -e EGIB -e naidnePF -e BIGE -e FPendian
EGIB
naidnePF
adler tmp #

@ghost
Copy link
Author

ghost commented Dec 25, 2020

Code blocks added, sorry, my bad.
I think the third "it" is refering to the script. I wonder about the "it" in "make sure it stays the same": what should be checked to stay the same?
BTW: I did some more checks, I'm pretty sure lto doesn't play nice with suspressed linking. So we need to drop "-c" and add "-shared". If this is done via patch or via sed shouldn't effect the outcome.

@leandrolnh
Copy link

I've came to the same issue while rebuilding world.
I ended up disabling LTO to this package in package.cflags with something like media-libs/x264 *FLAGS-="${FLTO}"
Then it passes endianess test.

@ghost
Copy link
Author

ghost commented Dec 28, 2020

I've no preference regarding patch or sed, but I dislike disabling lto just to make the configure script happy, though the package builds perfectly with lto. I strongly opt for dropping "-c" one way or another.

@jiblime
Copy link
Contributor

jiblime commented Dec 28, 2020

I've came to the same issue while rebuilding world.
I ended up disabling LTO to this package in package.cflags with something like media-libs/x264 *FLAGS-="${FLTO}"
Then it passes endianess test.

Adding -ffat-lto-objects also works

#602 (comment)

@pchome
Copy link
Contributor

pchome commented Dec 28, 2020

@nvertigo

I've no preference regarding patch or sed, but I dislike disabling lto just to make the configure script happy, though the package builds perfectly with lto.

This days no-one even try to figure out how to make LTO "happy", in any unusual case they just PR "fix" with -ffat-lto-objects or *FLAGS-="${FLTO}. That's why I have my own configuration file and avoid disabling lto if possible.

$ grep -E '^[^#].*\-="\-flto' /etc/portage/package.cflags/* | wc -l`
10

Original patch, BTW:
https://github.com/InBetweenNames/gentooLTO/pull/55/files#diff-66ee44970c5a708ec867ab37b73c7b2fdc73984ba39f3a8a92b050c46b41d8fb

I have no idea why they added -c back.

@ghost
Copy link
Author

ghost commented Jan 4, 2021

Fixed: 911a0a1

@ghost ghost closed this as completed Jan 4, 2021
This issue was closed.
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

No branches or pull requests

3 participants