generate_gypi.sh: --disable-avx and --only-config flags

generate_gypi.sh: --disable-avx and --only-config flags

Add two flags to make the script more flexible:
--disable-avx: do not generate AVX and AVX2 sources.
--only-configs: only generate configuration header (i.e. no GN or GYP files).

TESTED=Ran:
./generate_gypi.sh
and verified nothing changed in the checkout.
./generate_gypi.sh --disable-avx
and verified AVX was disabled.
./generate_gypi.sh --only-configs
with a modified .gypi and verified it was not overwritten.

R=johannkoenig@google.com

Committed: https://chromium.googlesource.com/chromium/deps/libvpx/+/46b04d71b9d97d2719c0d93937b049a6f72cfde1

[kjellander_chromium, 2015-09-14 09:57:27]

kjellander@chromium.org changed reviewers:
+ johannkoenig@chromium.org

[kjellander_chromium, 2015-09-14 09:57:28]

Would you be OK with adding something like this to the script? It doesn't change
any behavior, but it suits our needs in WebRTC better for now.

[kjellander_chromium, 2015-09-14 12:43:21]

So, it turns out that for PS#1 there are still AVX2 functions in the
configuration headers, even if --disable-avx is used for generate_gypi.h,
causing link errors.

In PS#2 I'm demonstrating the output of such a run, with some changes that are
out of my expertise to the
source/libvpx/vpx_dsp/vpx_dsp_rtcd_defs.pl file, which I assume has to be landed
as a CL for https://chromium.googlesource.com/webm/libvpx instead?
Can you take care of that for me, if this turns out to be something that should
be corrected like this? Then I can remove PS#2 from this CL.

[Johann, 2015-09-14 17:17:48]

On 2015/09/14 12:43:21, kjellander (chromium) wrote:
> So, it turns out that for PS#1 there are still AVX2 functions in the
> configuration headers, even if --disable-avx is used for generate_gypi.h,
> causing link errors.
> 
> In PS#2 I'm demonstrating the output of such a run, with some changes that are
> out of my expertise to the
> source/libvpx/vpx_dsp/vpx_dsp_rtcd_defs.pl file, which I assume has to be
landed
> as a CL for https://chromium.googlesource.com/webm/libvpx instead?
> Can you take care of that for me, if this turns out to be something that
should
> be corrected like this? Then I can remove PS#2 from this CL.

yeah --disable-avx2 is not passed between the configure and the rtcd generation.
ideally they'd be in the same step, but we call the rtcd generation directly
here.

[Johann, 2015-09-14 17:24:03]

johannkoenig@google.com changed reviewers:
+ johannkoenig@google.com

[Johann, 2015-09-14 17:24:04]

some options. generally looks fine

https://codereview.chromium.org/1339033002/diff/1/generate_gypi.sh
File generate_gypi.sh (right):

https://codereview.chromium.org/1339033002/diff/1/generate_gypi.sh#newcode25
generate_gypi.sh:25: 
maybe unset DISABLE_AVX?

https://codereview.chromium.org/1339033002/diff/1/generate_gypi.sh#newcode30
generate_gypi.sh:30: DISABLE_AVX=true
DISABLE_AVX="--disable-avx --disable-avx2"

https://codereview.chromium.org/1339033002/diff/1/generate_gypi.sh#newcode38
generate_gypi.sh:38: # Unknown option.
fail/error on unknown option

https://codereview.chromium.org/1339033002/diff/1/generate_gypi.sh#newcode141
generate_gypi.sh:141: if [[ ! $DISABLE_AVX && $4 == avx2 ]]; then
-nz or -ne or whatever "not set"

https://codereview.chromium.org/1339033002/diff/1/generate_gypi.sh#newcode366
generate_gypi.sh:366: --sym=vp8_rtcd \
add a line like

--sym=vp8_rtcd $DISABLE_AVX \

https://codereview.chromium.org/1339033002/diff/1/generate_gypi.sh#newcode373
generate_gypi.sh:373: --sym=vp9_rtcd \
need to do it for all the rtcd targets

https://codereview.chromium.org/1339033002/diff/1/generate_gypi.sh#newcode427
generate_gypi.sh:427: all_platforms="--enable-external-build --enable-postproc
--disable-install-srcs --enable-multi-res-encoding --enable-temporal-denoising
--disable-unit-tests --disable-install-docs --disable-examples
--enable-vp9-temporal-denoising --enable-vp9-postproc --size-limit=16384x16384"
add DISABLE_AVX to this line

https://codereview.chromium.org/1339033002/diff/20001/generate_gypi.sh
File generate_gypi.sh (right):

https://codereview.chromium.org/1339033002/diff/20001/generate_gypi.sh#newcod...
generate_gypi.sh:366: --sym=vp8_rtcd \
unfortunately the configure arguments are not passed here, so you need to add
--disable-avx2. see the change where we finally got around to turning it on:

https://chromium.googlesource.com/chromium/deps/libvpx/+/5cdd30205301c293be61...

[kjellander_chromium, 2015-09-14 18:38:21]

Thanks for the quick turnaround. PTAL @ PS#3.

https://codereview.chromium.org/1339033002/diff/1/generate_gypi.sh
File generate_gypi.sh (right):

https://codereview.chromium.org/1339033002/diff/1/generate_gypi.sh#newcode25
generate_gypi.sh:25: 
On 2015/09/14 17:24:04, Johann wrote:
> maybe unset DISABLE_AVX?

Done.

https://codereview.chromium.org/1339033002/diff/1/generate_gypi.sh#newcode30
generate_gypi.sh:30: DISABLE_AVX=true
On 2015/09/14 17:24:04, Johann wrote:
> DISABLE_AVX="--disable-avx --disable-avx2"

Done.

https://codereview.chromium.org/1339033002/diff/1/generate_gypi.sh#newcode38
generate_gypi.sh:38: # Unknown option.
On 2015/09/14 17:24:04, Johann wrote:
> fail/error on unknown option

Good idea, done.

https://codereview.chromium.org/1339033002/diff/1/generate_gypi.sh#newcode141
generate_gypi.sh:141: if [[ ! $DISABLE_AVX && $4 == avx2 ]]; then
On 2015/09/14 17:24:03, Johann wrote:
> -nz or -ne or whatever "not set"

Done.

https://codereview.chromium.org/1339033002/diff/1/generate_gypi.sh#newcode366
generate_gypi.sh:366: --sym=vp8_rtcd \
On 2015/09/14 17:24:04, Johann wrote:
> add a line like
> 
> --sym=vp8_rtcd $DISABLE_AVX \

Done.

https://codereview.chromium.org/1339033002/diff/1/generate_gypi.sh#newcode373
generate_gypi.sh:373: --sym=vp9_rtcd \
On 2015/09/14 17:24:04, Johann wrote:
> need to do it for all the rtcd targets

Done.

https://codereview.chromium.org/1339033002/diff/1/generate_gypi.sh#newcode427
generate_gypi.sh:427: all_platforms="--enable-external-build --enable-postproc
--disable-install-srcs --enable-multi-res-encoding --enable-temporal-denoising
--disable-unit-tests --disable-install-docs --disable-examples
--enable-vp9-temporal-denoising --enable-vp9-postproc --size-limit=16384x16384"
On 2015/09/14 17:24:04, Johann wrote:
> add DISABLE_AVX to this line

Done.

[Johann, 2015-09-14 18:51:02]

https://codereview.chromium.org/1339033002/diff/40001/source/libvpx/vpx_dsp/v...
File source/libvpx/vpx_dsp/vpx_dsp_rtcd_defs.pl (right):

https://codereview.chromium.org/1339033002/diff/40001/source/libvpx/vpx_dsp/v...
source/libvpx/vpx_dsp/vpx_dsp_rtcd_defs.pl:15: $avx2 = '';
this shouldn't be necessary anymore? although this is an interesting approach,
would be good of us to use vpx_config directly.

[Johann, 2015-09-14 19:07:18]

fyi I just landed something in DEPS so you should rebase, but I didn't touch
generate_gypi.sh so it should be trivial. also for some reason my changes get
uploaded to chromium-review instead of codereview. weird.

[kjellander_chromium, 2015-09-14 19:40:45]

Now rebased and simpler. PTAL.

https://codereview.chromium.org/1339033002/diff/40001/source/libvpx/vpx_dsp/v...
File source/libvpx/vpx_dsp/vpx_dsp_rtcd_defs.pl (right):

https://codereview.chromium.org/1339033002/diff/40001/source/libvpx/vpx_dsp/v...
source/libvpx/vpx_dsp/vpx_dsp_rtcd_defs.pl:15: $avx2 = '';
On 2015/09/14 18:51:02, Johann wrote:
> this shouldn't be necessary anymore? although this is an interesting approach,
> would be good of us to use vpx_config directly.

Ah, you're right. That's nice. It seemed a bit ugly.

[chromium-reviews, 2015-09-14 20:01:33]

LGTM thanks for the follow ups!

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[kjellander_chromium, 2015-09-14 20:04:30]

On 2015/09/14 20:01:33, chromium-reviews wrote:
> LGTM thanks for the follow ups!
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

I need your chromium.org account approval. Not sure why you have problems with
chromium-reviews...

[Johann, 2015-09-14 20:47:17]

On 2015/09/14 20:04:30, kjellander (chromium) wrote:
> On 2015/09/14 20:01:33, chromium-reviews wrote:
> > LGTM thanks for the follow ups!
> > 
> > To unsubscribe from this group and stop receiving emails from it, send an
> email
> > to mailto:chromium-reviews+unsubscribe@chromium.org.
> 
> I need your http://chromium.org account approval. Not sure why you have
problems with
> chromium-reviews...

LGTM

I sent the previous reply via email because I was on my phone. Lets see what
happens now ...

[kjellander_chromium, 2015-09-15 09:28:48]

Committed patchset #5 (id:80001) manually as
46b04d71b9d97d2719c0d93937b049a6f72cfde1 (presubmit successful).