Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 594634 - media-libs/libvpx: -O3 flag is not stripped
Summary: media-libs/libvpx: -O3 flag is not stripped
Status: UNCONFIRMED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: media-video herd
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-09-21 15:44 UTC by .
Modified: 2017-07-20 19:13 UTC (History)
2 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
disable optimizations (file_594634.txt,470 bytes, patch)
2017-07-20 13:23 UTC, Steve Dibb
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description . 2016-09-21 15:44:52 UTC
Example:

```
mkdir -p vp9/encoder/
x86_64-pc-linux-gnu-gcc  -m64 -DNDEBUG -O3 -fPIC -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -Wall -Wdeclaration-after-statement -Wdisabled-optimization -Wpointer-arith -Wtype-limits -Wcast-qual -Wvla -Wimplicit-function-declaration -Wuninitialized -Wunused-variable -Wunused-but-set-variable -Wunused-function -I. -I"/tmp/portage/media-libs/libvpx-1.6.0-r1/work/libvpx-1.6.0" -msse2 -M /tmp/portage/media-libs/libvpx-1.6.0-r1/work/libvpx-1.6.0/vp9/encoder/x86/vp9_quantize_sse2.c | sed -e 's;^\([a-zA-Z0-9_]*\)\.o;vp9/encoder/x86/vp9_quantize_sse2.c.o vp9/encoder/x86/vp9_quantize_sse2.c.d;' > vp9/encoder/x86/vp9_quantize_sse2.c.d
x86_64-pc-linux-gnu-gcc  -m64 -DNDEBUG -O3 -fPIC -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -Wall -Wdeclaration-after-statement -Wdisabled-optimization -Wpointer-arith -Wtype-limits -Wcast-qual -Wvla -Wimplicit-function-declaration -Wuninitialized -Wunused-variable -Wunused-but-set-variable -Wunused-function -I. -I"/tmp/portage/media-libs/libvpx-1.6.0-r1/work/libvpx-1.6.0" -M /tmp/portage/media-libs/libvpx-1.6.0-r1/work/libvpx-1.6.0/vp9/encoder/vp9_mbgraph.c | sed -e 's;^\([a-zA-Z0-9_]*\)\.o;vp9/encoder/vp9_mbgraph.c.o vp9/encoder/vp9_mbgraph.c.d;' > vp9/encoder/vp9_mbgraph.c.d
```
Comment 1 Steve Dibb 2017-07-20 13:20:11 UTC
build/make/configure.sh is adding them, enabling optimizations is set by default.

The ebuild could be updated to run the --disable-optimizations and just let us use our own, or we could add an optimization USE flag, and then let them set it.

Either way, they add a lot of stuff to cflags on their own, so could be a slippery slope.
Comment 2 Steve Dibb 2017-07-20 13:23:38 UTC
Created attachment 486018 [details, diff]
disable optimizations
Comment 3 Mike Gilbert gentoo-dev 2017-07-20 14:16:43 UTC
(In reply to Steve Dibb from comment #2)
> Created attachment 486018 [details, diff] [details, diff]
> disable optimizations

This patch seems reasonable to me.
Comment 4 Alexis Ballier gentoo-dev 2017-07-20 17:44:07 UTC
what do they add as cflags ?

it'll probably work on x86* but e.g. altivec or neon asm usually fails at the 'as call' stage if not used with proper flags iirc
Comment 5 Steve Dibb 2017-07-20 18:32:38 UTC
(In reply to Alexis Ballier from comment #4)
> what do they add as cflags ?

Turns on at least -O2:

  if enabled optimizations; then
    if enabled rvct; then
      enabled small && check_add_cflags -Ospace || check_add_cflags -Otime
    else
      enabled small && check_add_cflags -O2 ||  check_add_cflags -O3
    fi
  fi

rvct is for armv6, armv7

"enabled small" is another configure flag that can be passed, --enable-small, and this is the only place it's used.
 
> it'll probably work on x86* but e.g. altivec or neon asm usually fails at
> the 'as call' stage if not used with proper flags iirc

There's other stuff in here for neon asm as well, but I don't know what it does.

One example:

            if enabled neon || enabled neon_asm; then
              check_add_cflags -mfpu=neon #-ftree-vectorize
              check_add_asflags -mfpu=neon
            fi
Comment 6 Steve Dibb 2017-07-20 18:38:46 UTC
Here's the last place it uses the optimizations configuration:

        gcc*)
          link_with_cc=gcc
          tune_cflags="-march="
          setup_gnu_toolchain
          #for 32 bit x86 builds, -O3 did not turn on this flag
          enabled optimizations && disabled gprof && check_add_cflags -fomit-frame-pointer
          ;;

gprof is "gprof profiling instrumentation", which is also not enabled by default.

So the question is, whose CFLAGS to accept? It's always a tricky issue with codecs, and I generally fall in favor with accepting upstream's recommendations. We could throw in an optimizations USE flag or custom-cflags and strip it all, but I see too much changing it to just cause problems down the road for someone at the expense of them thinking they're making things better.

My recommendation: strip -Os and add optimizations use flag.
Comment 7 Alexis Ballier gentoo-dev 2017-07-20 18:40:11 UTC
(In reply to Steve Dibb from comment #5)
> (In reply to Alexis Ballier from comment #4)
> > what do they add as cflags ?
> 
> Turns on at least -O2:
> 
>   if enabled optimizations; then
>     if enabled rvct; then
>       enabled small && check_add_cflags -Ospace || check_add_cflags -Otime
>     else
>       enabled small && check_add_cflags -O2 ||  check_add_cflags -O3
>     fi
>   fi


if that's the only place this has any effect then it definitely makes sense to disable optimizations
Comment 8 Alexis Ballier gentoo-dev 2017-07-20 18:41:48 UTC
(In reply to Steve Dibb from comment #6)
> So the question is, whose CFLAGS to accept? It's always a tricky issue with
> codecs, and I generally fall in favor with accepting upstream's
> recommendations. We could throw in an optimizations USE flag or
> custom-cflags and strip it all, but I see too much changing it to just cause
> problems down the road for someone at the expense of them thinking they're
> making things better.


Nah, I think your patch is the way to go then: I see very little value of a CFLAGS from upstream only adding -O2 or -O3, taking into consideration that what those flags mean change a lot between gcc versions.
Comment 9 Steve Dibb 2017-07-20 19:13:51 UTC
(In reply to Alexis Ballier from comment #8)
> (In reply to Steve Dibb from comment #6)
> > So the question is, whose CFLAGS to accept? It's always a tricky issue with
> > codecs, and I generally fall in favor with accepting upstream's
> > recommendations. We could throw in an optimizations USE flag or
> > custom-cflags and strip it all, but I see too much changing it to just cause
> > problems down the road for someone at the expense of them thinking they're
> > making things better.
> 
> 
> Nah, I think your patch is the way to go then: I see very little value of a
> CFLAGS from upstream only adding -O2 or -O3, taking into consideration that
> what those flags mean change a lot between gcc versions.

That works.

Looked at 1.6.0 and 1.6.1 build systems, and there's no additional changes to the configure.sh script compared to 1.5.0.