diff mbox series

[FFmpeg-devel] configure: fix the bigendian test

Message ID 87imaclyzp.fsf@gmail.com
State Superseded
Headers show
Series [FFmpeg-devel] configure: fix the bigendian test | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

David Michael Nov. 11, 2020, 3 p.m. UTC
There are two issues:

The unused global integer does not make it into the compiled object
file, so declare it static to correct this.

The hexdump output is line-wrapped, so the expected value will not
be matched when its four bytes are split across two lines.  Use the
POSIX "-A n" option to disable printing input offsets and delete
all newline characters to output continuous hex values to grep.

Signed-off-by: David Michael <fedora.dm0@gmail.com>
---
 configure | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Martin Storsjö Nov. 11, 2020, 10:18 p.m. UTC | #1
On Wed, 11 Nov 2020, David Michael wrote:

> There are two issues:
>
> The unused global integer does not make it into the compiled object
> file, so declare it static to correct this.

Umm, what? That sounds entirely counterintuitive to me.

If the global integer is non-static, it could be referred to by another 
object file, so the compiler cannot omit it from the generated object 
file. On the other side, if the variable is static, the compiler has full 
visibility over any possible use, and sees that nothing in the same object 
file refers to it, and thus can omit it if it's unused.

// Martin
David Michael Nov. 11, 2020, 10:25 p.m. UTC | #2
On Wed, Nov 11, 2020 at 5:18 PM Martin Storsjö <martin@martin.st> wrote:
> On Wed, 11 Nov 2020, David Michael wrote:
> > There are two issues:
> >
> > The unused global integer does not make it into the compiled object
> > file, so declare it static to correct this.
>
> Umm, what? That sounds entirely counterintuitive to me.
>
> If the global integer is non-static, it could be referred to by another
> object file, so the compiler cannot omit it from the generated object
> file. On the other side, if the variable is static, the compiler has full
> visibility over any possible use, and sees that nothing in the same object
> file refers to it, and thus can omit it if it's unused.

LTO removes it if it is not static.  It is always present when it is static.

Thanks.

David
Carl Eugen Hoyos Nov. 11, 2020, 11:19 p.m. UTC | #3
Am Mi., 11. Nov. 2020 um 16:30 Uhr schrieb David Michael <fedora.dm0@gmail.com>:
>
> There are two issues:
>
> The unused global integer does not make it into the compiled object
> file, so declare it static to correct this.

How can I reproduce this issue?

> The hexdump output is line-wrapped, so the expected value will not
> be matched when its four bytes are split across two lines.  Use the
> POSIX "-A n" option to disable printing input offsets and delete
> all newline characters to output continuous hex values to grep.

Will this work on AIX?

Carl Eugen
David Michael Nov. 11, 2020, 11:41 p.m. UTC | #4
On Wed, Nov 11, 2020 at 6:20 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> Am Mi., 11. Nov. 2020 um 16:30 Uhr schrieb David Michael <fedora.dm0@gmail.com>:
> > There are two issues:
> >
> > The unused global integer does not make it into the compiled object
> > file, so declare it static to correct this.
>
> How can I reproduce this issue?

The configure test is basically these lines (where I use a PowerPC GCC
for a big-endian compiler):

echo "unsigned int endian = 'B' << 24 | 'I' << 16 | 'G' << 8 | 'E';" |
powerpc-gentoo-linux-gnu-gcc $CFLAGS -c -o test.o -xc - &&
od -A n -t x1 test.o | tr -d '\n' | grep -o '42 *49 *47 *45'

In testing this just now, it seems that the static value is only
present when "-O2 -g" is in CFLAGS, which I was always using as the
bare minimum flags from various distro packaging environments.  So
without static, the test is broken with "-flto"; with static, the test
is broken without "-O2 -g".  Maybe it could just check for a
_BIG_ENDIAN preprocessor definition instead.

> > The hexdump output is line-wrapped, so the expected value will not
> > be matched when its four bytes are split across two lines.  Use the
> > POSIX "-A n" option to disable printing input offsets and delete
> > all newline characters to output continuous hex values to grep.
>
> Will this work on AIX?

I don't have access to test it, but the behavior is described in the
Open Group specification, so I assume it would.

Thanks.

David
Carl Eugen Hoyos Nov. 12, 2020, 7:30 a.m. UTC | #5
> Am 12.11.2020 um 00:41 schrieb David Michael <fedora.dm0@gmail.com>:
> 
>> On Wed, Nov 11, 2020 at 6:20 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>>> Am Mi., 11. Nov. 2020 um 16:30 Uhr schrieb David Michael <fedora.dm0@gmail.com>:
>>> There are two issues:
>>> 
>>> The unused global integer does not make it into the compiled object
>>> file, so declare it static to correct this.
>> 
>> How can I reproduce this issue?
> 
> The configure test is basically these lines (where I use a PowerPC GCC
> for a big-endian compiler):
> 
> echo "unsigned int endian = 'B' << 24 | 'I' << 16 | 'G' << 8 | 'E';" |
> powerpc-gentoo-linux-gnu-gcc $CFLAGS -c -o test.o -xc - &&
> od -A n -t x1 test.o | tr -d '\n' | grep -o '42 *49 *47 *45'
> 
> In testing this just now, it seems that the static value is only
> present when "-O2 -g" is in CFLAGS, which I was always using as the
> bare minimum flags from various distro packaging environments.  So
> without static, the test is broken with "-flto"; with static, the test
> is broken without "-O2 -g".  Maybe it could just check for a
> _BIG_ENDIAN preprocessor definition instead.

Sorry for the misunderstanding:
What do I have to do (on the command line) to reproduce a problem with current FFmpeg git head?

Carl Eugen
David Michael Nov. 12, 2020, 1:43 p.m. UTC | #6
On Thu, Nov 12, 2020 at 2:56 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> > Am 12.11.2020 um 00:41 schrieb David Michael <fedora.dm0@gmail.com>:
> >> On Wed, Nov 11, 2020 at 6:20 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> >>> Am Mi., 11. Nov. 2020 um 16:30 Uhr schrieb David Michael <fedora.dm0@gmail.com>:
> >>> There are two issues:
> >>>
> >>> The unused global integer does not make it into the compiled object
> >>> file, so declare it static to correct this.
> >>
> >> How can I reproduce this issue?
> >
> > The configure test is basically these lines (where I use a PowerPC GCC
> > for a big-endian compiler):
> >
> > echo "unsigned int endian = 'B' << 24 | 'I' << 16 | 'G' << 8 | 'E';" |
> > powerpc-gentoo-linux-gnu-gcc $CFLAGS -c -o test.o -xc - &&
> > od -A n -t x1 test.o | tr -d '\n' | grep -o '42 *49 *47 *45'
> >
> > In testing this just now, it seems that the static value is only
> > present when "-O2 -g" is in CFLAGS, which I was always using as the
> > bare minimum flags from various distro packaging environments.  So
> > without static, the test is broken with "-flto"; with static, the test
> > is broken without "-O2 -g".  Maybe it could just check for a
> > _BIG_ENDIAN preprocessor definition instead.
>
> Sorry for the misunderstanding:
> What do I have to do (on the command line) to reproduce a problem with current FFmpeg git head?

Call the configure script to cross-compile to a big-endian
architecture with CFLAGS in the environment.  Basically run this with
your own compiler prefix:

./configure --enable-cross-compile --host-cc=gcc --arch=powerpc
--target-os=linux --cross-prefix=powerpc-unknown-linux-gnu-

If CFLAGS contains -flto, you'll see "big-endian no" in the output.
That results in compilation failures later due to missing PPC
definitions that were conditional on HAVE_BIGENDIAN.

Thanks.

David
diff mbox series

Patch

diff --git a/configure b/configure
index 2f02d7f5c8..52c02a74c8 100755
--- a/configure
+++ b/configure
@@ -5749,9 +5749,9 @@  check_cc pragma_deprecated "" '_Pragma("GCC diagnostic ignored \"-Wdeprecated-de
 
 # The global variable ensures the bits appear unchanged in the object file.
 test_cc <<EOF || die "endian test failed"
-unsigned int endian = 'B' << 24 | 'I' << 16 | 'G' << 8 | 'E';
+static unsigned int endian = 'B' << 24 | 'I' << 16 | 'G' << 8 | 'E';
 EOF
-od -t x1 $TMPO | grep -q '42 *49 *47 *45' && enable bigendian
+od -A n -t x1 $TMPO | tr -d '\n' | grep -q '42 *49 *47 *45' && enable bigendian
 
 check_cc const_nan math.h "struct { double d; } static const bar[] = { { NAN } }"