[GN] Add support for disabling opts via SK_BUILD_NO_OPTS define.

[GN] Add support for disabling opts via SK_BUILD_NO_OPTS define.

When targetting iOS and using gyp to generate the build files, it is not
possible to select files to build depending on the architecture. Due to
that, the skia code was disabling all optimisation when SK_BUILD_FOR_IOS
was defined.

Since it is possible to select the correct optimised version when using
gn, this pessimisation is hurting the build. Introduce a new define to
disable the optimisation SK_BUILD_NO_OPTS. It will be used by Chromium
when building skia for iOS with gyp but not gn.

Define SK_BUILD_NO_OPTS along-side SK_BUILD_FOR_IOS for all files that
look like build configuration (Xcode projects, gyp configuration files,
public.bzl) in order to avoid introducing breakage on those builds.

BUG=607933
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2002423002

Committed: https://skia.googlesource.com/skia/+/e3fa811657ecf4ab694d026752a81080c6b10611

[sdefresne, 2016-05-24 09:31:02]

Description was changed from

==========
[GN] Add support for disabling opts via SK_BUILD_NO_OPTS define.

When targetting iOS and using gyp to generate the build files, it is not
possible to select files to build depending on the architecture. Due to
that, the skia code was disabling all optimisation when SK_BUILD_FOR_IOS
was defined.

Since it is possible to select the correct optimised version when using
gn, this pessimisation is hurting the build. Introduce a new define to
disable the optimisation SK_BUILD_NO_OPTS. It will be used by Chromium
when building skia for iOS with gyp but not gn.

Define SK_BUILD_NO_OPTS along-side SK_BUILD_FOR_IOS for all files that
look like build configuration (Xcode projects, gyp configuration files,
public.bzl) in order to avoid introducing breakage on those builds.

BUG=607933
==========

to

==========
[GN] Add support for disabling opts via SK_BUILD_NO_OPTS define.

When targetting iOS and using gyp to generate the build files, it is not
possible to select files to build depending on the architecture. Due to
that, the skia code was disabling all optimisation when SK_BUILD_FOR_IOS
was defined.

Since it is possible to select the correct optimised version when using
gn, this pessimisation is hurting the build. Introduce a new define to
disable the optimisation SK_BUILD_NO_OPTS. It will be used by Chromium
when building skia for iOS with gyp but not gn.

Define SK_BUILD_NO_OPTS along-side SK_BUILD_FOR_IOS for all files that
look like build configuration (Xcode projects, gyp configuration files,
public.bzl) in order to avoid introducing breakage on those builds.

BUG=607933
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2002423002
==========

[sdefresne, 2016-05-24 09:31:26]

Description was changed from

==========
[GN] Add support for disabling opts via SK_BUILD_NO_OPTS define.

When targetting iOS and using gyp to generate the build files, it is not
possible to select files to build depending on the architecture. Due to
that, the skia code was disabling all optimisation when SK_BUILD_FOR_IOS
was defined.

Since it is possible to select the correct optimised version when using
gn, this pessimisation is hurting the build. Introduce a new define to
disable the optimisation SK_BUILD_NO_OPTS. It will be used by Chromium
when building skia for iOS with gyp but not gn.

Define SK_BUILD_NO_OPTS along-side SK_BUILD_FOR_IOS for all files that
look like build configuration (Xcode projects, gyp configuration files,
public.bzl) in order to avoid introducing breakage on those builds.

BUG=607933
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2002423002
==========

to

==========
[GN] Add support for disabling opts via SK_BUILD_NO_OPTS define.

When targetting iOS and using gyp to generate the build files, it is not
possible to select files to build depending on the architecture. Due to
that, the skia code was disabling all optimisation when SK_BUILD_FOR_IOS
was defined.

Since it is possible to select the correct optimised version when using
gn, this pessimisation is hurting the build. Introduce a new define to
disable the optimisation SK_BUILD_NO_OPTS. It will be used by Chromium
when building skia for iOS with gyp but not gn.

Define SK_BUILD_NO_OPTS along-side SK_BUILD_FOR_IOS for all files that
look like build configuration (Xcode projects, gyp configuration files,
public.bzl) in order to avoid introducing breakage on those builds.

BUG=607933
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2002423002
==========

[sdefresne, 2016-05-24 09:31:26]

sdefresne@chromium.org changed reviewers:
+ bsalomon@google.com

[sdefresne, 2016-05-24 09:33:15]

Please take a look.

This needs https://codereview.chromium.org/2008853002/ to land first in Chromium
repository. Once both CLs lands and Chromium roll skia, we can finally have
optimised code on iOS.

[mtklein, 2016-05-24 12:33:26]

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

[mtklein, 2016-05-24 12:33:26]

Are iOS GYP and GN builds going to both be active for long (more than a year or
so)?  If not, why don't we just circle back to iOS opts after GYP is gone?

[sdefresne, 2016-05-24 13:37:09]

On 2016/05/24 12:33:26, mtklein wrote:
> Are iOS GYP and GN builds going to both be active for long (more than a year
or
> so)?  If not, why don't we just circle back to iOS opts after GYP is gone?

The plan is to switch Chromium from gyp to gn as soon as possible (at most a
couple of releases).

However, before making the switch we need to have both gyp and gn building.
Without this CL, skia does not build on iOS. Since the target_cpu is correctly
set when building for iOS using gn (but not when using gyp), the skia build
enables the compilation of optimised version, e.g. SkBlitRow_opts_arm_neon.cpp
but the file do not build because SK_CPU_ARM64 is not defined. This cause armv7
asm instruction to be used which apparently are not supported and cause the
following error:

../../third_party/skia/src/opts/SkBlitRow_opts_arm_neon.cpp:132:13: error:
vector register expected
            "vld4.8    {d0-d3},[%[src]]!"
            ^

I see the two possible options:
1. align gn with gyp (i.e. disable optimisations),
2. use a define to disable optimisation and use it with gyp.

I think option 2. is better because if I implements option 1., once we switch to
gn, I'll have to revert the changes, and then fix skia to enable optimisation
(i.e. basically do what I've done here). If you want I can create a bug to
remove the SK_BUILD_NO_OPTS define as soon as the switch to gn is in place (as a
said should happen in a couple releases at most).

[mtklein, 2016-05-24 13:46:39]

On 2016/05/24 13:37:09, sdefresne wrote:
> On 2016/05/24 12:33:26, mtklein wrote:
> > Are iOS GYP and GN builds going to both be active for long (more than a year
> or
> > so)?  If not, why don't we just circle back to iOS opts after GYP is gone?
> 
> The plan is to switch Chromium from gyp to gn as soon as possible (at most a
> couple of releases).
> 
> However, before making the switch we need to have both gyp and gn building.
> Without this CL, skia does not build on iOS. Since the target_cpu is correctly
> set when building for iOS using gn (but not when using gyp), the skia build
> enables the compilation of optimised version, e.g. SkBlitRow_opts_arm_neon.cpp
> but the file do not build because SK_CPU_ARM64 is not defined. This cause
armv7
> asm instruction to be used which apparently are not supported and cause the
> following error:
> 
> ../../third_party/skia/src/opts/SkBlitRow_opts_arm_neon.cpp:132:13: error:
> vector register expected
>             "vld4.8    {d0-d3},[%[src]]!"
>             ^
> 
> I see the two possible options:
> 1. align gn with gyp (i.e. disable optimisations),
> 2. use a define to disable optimisation and use it with gyp.
> 
> I think option 2. is better because if I implements option 1., once we switch
to
> gn, I'll have to revert the changes, and then fix skia to enable optimisation
> (i.e. basically do what I've done here). If you want I can create a bug to
> remove the SK_BUILD_NO_OPTS define as soon as the switch to gn is in place (as
a
> said should happen in a couple releases at most).

Yes, option 1 is more work, and it's the safer choice.  Option 2 couples two
risky changes: GYP -> GN, and portable code -> ARM-specific code.  We have never
tested platform-specific code on iOS.  You may not want to bet your GN
transition on that code working.

[sdefresne, 2016-05-24 15:36:23]

On 2016/05/24 13:46:39, mtklein wrote:
> On 2016/05/24 13:37:09, sdefresne wrote:
> > On 2016/05/24 12:33:26, mtklein wrote:
> > > Are iOS GYP and GN builds going to both be active for long (more than a
year
> > or
> > > so)?  If not, why don't we just circle back to iOS opts after GYP is gone?
> > 
> > The plan is to switch Chromium from gyp to gn as soon as possible (at most a
> > couple of releases).
> > 
> > However, before making the switch we need to have both gyp and gn building.
> > Without this CL, skia does not build on iOS. Since the target_cpu is
correctly
> > set when building for iOS using gn (but not when using gyp), the skia build
> > enables the compilation of optimised version, e.g.
SkBlitRow_opts_arm_neon.cpp
> > but the file do not build because SK_CPU_ARM64 is not defined. This cause
> armv7
> > asm instruction to be used which apparently are not supported and cause the
> > following error:
> > 
> > ../../third_party/skia/src/opts/SkBlitRow_opts_arm_neon.cpp:132:13: error:
> > vector register expected
> >             "vld4.8    {d0-d3},[%[src]]!"
> >             ^
> > 
> > I see the two possible options:
> > 1. align gn with gyp (i.e. disable optimisations),
> > 2. use a define to disable optimisation and use it with gyp.
> > 
> > I think option 2. is better because if I implements option 1., once we
switch
> to
> > gn, I'll have to revert the changes, and then fix skia to enable
optimisation
> > (i.e. basically do what I've done here). If you want I can create a bug to
> > remove the SK_BUILD_NO_OPTS define as soon as the switch to gn is in place
(as
> a
> > said should happen in a couple releases at most).
> 
> Yes, option 1 is more work, and it's the safer choice.  Option 2 couples two
> risky changes: GYP -> GN, and portable code -> ARM-specific code.  We have
never
> tested platform-specific code on iOS.  You may not want to bet your GN
> transition on that code working.

OK, I'll go towards option 1. However, we should consider this CL. Option 1.
just mean that https://codereview.chromium.org/2008853002/ will also define
SK_BUILD_NO_OPTS when building with GN in addition to using none_sources. Then
it will be easier to test using optimisation on iOS (I can just flip the flag
and check that everything work).

WDYT?

[sdefresne, 2016-05-24 15:52:20]

On 2016/05/24 15:36:23, sdefresne wrote:
> On 2016/05/24 13:46:39, mtklein wrote:
> > On 2016/05/24 13:37:09, sdefresne wrote:
> > > On 2016/05/24 12:33:26, mtklein wrote:
> > > > Are iOS GYP and GN builds going to both be active for long (more than a
> year
> > > or
> > > > so)?  If not, why don't we just circle back to iOS opts after GYP is
gone?
> > > 
> > > The plan is to switch Chromium from gyp to gn as soon as possible (at most
a
> > > couple of releases).
> > > 
> > > However, before making the switch we need to have both gyp and gn
building.
> > > Without this CL, skia does not build on iOS. Since the target_cpu is
> correctly
> > > set when building for iOS using gn (but not when using gyp), the skia
build
> > > enables the compilation of optimised version, e.g.
> SkBlitRow_opts_arm_neon.cpp
> > > but the file do not build because SK_CPU_ARM64 is not defined. This cause
> > armv7
> > > asm instruction to be used which apparently are not supported and cause
the
> > > following error:
> > > 
> > > ../../third_party/skia/src/opts/SkBlitRow_opts_arm_neon.cpp:132:13: error:
> > > vector register expected
> > >             "vld4.8    {d0-d3},[%[src]]!"
> > >             ^
> > > 
> > > I see the two possible options:
> > > 1. align gn with gyp (i.e. disable optimisations),
> > > 2. use a define to disable optimisation and use it with gyp.
> > > 
> > > I think option 2. is better because if I implements option 1., once we
> switch
> > to
> > > gn, I'll have to revert the changes, and then fix skia to enable
> optimisation
> > > (i.e. basically do what I've done here). If you want I can create a bug to
> > > remove the SK_BUILD_NO_OPTS define as soon as the switch to gn is in place
> (as
> > a
> > > said should happen in a couple releases at most).
> > 
> > Yes, option 1 is more work, and it's the safer choice.  Option 2 couples two
> > risky changes: GYP -> GN, and portable code -> ARM-specific code.  We have
> never
> > tested platform-specific code on iOS.  You may not want to bet your GN
> > transition on that code working.
> 
> OK, I'll go towards option 1. However, we should consider this CL. Option 1.
> just mean that https://codereview.chromium.org/2008853002/ will also define
> SK_BUILD_NO_OPTS when building with GN in addition to using none_sources. Then
> it will be easier to test using optimisation on iOS (I can just flip the flag
> and check that everything work).
> 
> WDYT?

Updated Chromium CL https://codereview.chromium.org/2008853002/.

As the updated CL use SK_BUILD_NO_OPTS when using both gyp and gn, this should
prevent regression. I'd still like to land this skia CL though as it will make
easier later to re-enable optimisation on iOS (just flip a boolean in
skia/BUILD.gn in Chromium source tree) and it will also make skia code easier to
understand (there are comments near the !defined(SK_BUILD_FOR_IOS) asking why
the optimisation are disabled on iOS at least in two locations).

[mtklein_C, 2016-05-24 15:59:14]

mtklein@chromium.org changed reviewers:
+ mtklein@chromium.org

[mtklein_C, 2016-05-24 15:59:15]

lgtm

This plan sounds good to me.

I've also realized that this allows us to start testing iOS opts on our bots
ahead of Chrome, by conditionally defining that flag in our GYP.  That's a nice
path forward... that'll let us get testing in parallel.

[sdefresne, 2016-05-24 16:03:02]

On 2016/05/24 15:59:15, mtklein_C wrote:
> lgtm
> 
> This plan sounds good to me.
> 
> I've also realized that this allows us to start testing iOS opts on our bots
> ahead of Chrome, by conditionally defining that flag in our GYP.  That's a
nice
> path forward... that'll let us get testing in parallel.

Thanks. Waiting for bsalomon OWNERS review.

[sdefresne, 2016-05-28 12:31:50]

bsalomon: ping?

[sdefresne, 2016-06-01 07:53:17]

sdefresne@chromium.org changed reviewers:
+ reed@google.com
- bsalomon@google.com, mtklein@google.com

[sdefresne, 2016-06-01 07:54:22]

-bsalomon, +reed: changing OWNERS as bsalomon is unresponsive.

reed: can you do an OWNERS review? This has already been reviewed by mtklein and
should enable use to have optimised skia code on iOS eventually.

[reed1, 2016-06-01 13:46:05]

lgtm

[sdefresne, 2016-06-01 13:50:18]

On 2016/06/01 13:46:05, reed1 wrote:
> lgtm

Thank you. How should I land this CL? Can I just use the CQ? This is my first CL
against skia.

[reed1, 2016-06-01 13:54:28]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2016-06-01 13:54:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2002423002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2002423002/1

[commit-bot: I haz the power, 2016-06-01 14:09:00]

Description was changed from

==========
[GN] Add support for disabling opts via SK_BUILD_NO_OPTS define.

When targetting iOS and using gyp to generate the build files, it is not
possible to select files to build depending on the architecture. Due to
that, the skia code was disabling all optimisation when SK_BUILD_FOR_IOS
was defined.

Since it is possible to select the correct optimised version when using
gn, this pessimisation is hurting the build. Introduce a new define to
disable the optimisation SK_BUILD_NO_OPTS. It will be used by Chromium
when building skia for iOS with gyp but not gn.

Define SK_BUILD_NO_OPTS along-side SK_BUILD_FOR_IOS for all files that
look like build configuration (Xcode projects, gyp configuration files,
public.bzl) in order to avoid introducing breakage on those builds.

BUG=607933
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2002423002
==========

to

==========
[GN] Add support for disabling opts via SK_BUILD_NO_OPTS define.

When targetting iOS and using gyp to generate the build files, it is not
possible to select files to build depending on the architecture. Due to
that, the skia code was disabling all optimisation when SK_BUILD_FOR_IOS
was defined.

Since it is possible to select the correct optimised version when using
gn, this pessimisation is hurting the build. Introduce a new define to
disable the optimisation SK_BUILD_NO_OPTS. It will be used by Chromium
when building skia for iOS with gyp but not gn.

Define SK_BUILD_NO_OPTS along-side SK_BUILD_FOR_IOS for all files that
look like build configuration (Xcode projects, gyp configuration files,
public.bzl) in order to avoid introducing breakage on those builds.

BUG=607933
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2002423002

Committed:
https://skia.googlesource.com/skia/+/e3fa811657ecf4ab694d026752a81080c6b10611
==========

[commit-bot: I haz the power, 2016-06-01 14:09:01]

Committed patchset #1 (id:1) as
https://skia.googlesource.com/skia/+/e3fa811657ecf4ab694d026752a81080c6b10611