change mapRectScaleTranslate to pass args/ret by value

change mapRectScaleTranslate to pass args/ret by value

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

TBR=mtklein

Committed: https://skia.googlesource.com/skia/+/14dce6ed5934d7a6e1fac79f8e76e12f5670aae2

[reed1, 2016-07-10 18:18:23]

Description was changed from

==========
change mapRectScaleTranslate to pass args/ret by value

BUG=skia:
==========

to

==========
change mapRectScaleTranslate to pass args/ret by value

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

[reed1, 2016-07-10 18:18:33]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-07-10 18:18:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[reed1, 2016-07-10 18:20:59]

Description was changed from

==========
change mapRectScaleTranslate to pass args/ret by value

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

to

==========
change mapRectScaleTranslate to pass args/ret by value

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

TBR=mtklein
==========

[reed1, 2016-07-10 18:21:18]

reed@google.com changed reviewers:
+ msarett@google.com, mtklein@google.com

[reed1, 2016-07-10 18:21:26]

The CQ bit was unchecked by reed@google.com

[reed1, 2016-07-10 18:21:57]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2016-07-10 18:22:05]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-10 18:35:06]

Description was changed from

==========
change mapRectScaleTranslate to pass args/ret by value

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

TBR=mtklein
==========

to

==========
change mapRectScaleTranslate to pass args/ret by value

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

TBR=mtklein

Committed:
https://skia.googlesource.com/skia/+/14dce6ed5934d7a6e1fac79f8e76e12f5670aae2
==========

[commit-bot: I haz the power, 2016-07-10 18:35:07]

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

[reed1, 2016-07-10 18:45:24]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/2139523002/ by reed@google.com.

The reason for reverting is: Triggered a compiler bug on Android?

FAILED: /usr/bin/ccache
/b/work/skia/platform_tools/android/bin/../toolchains/arm-r11c-14/bin/arm-linux-androideabi-g++
-MMD -MF obj/src/core/core.SkMatrix.o.d -DSK_INTERNAL -DSK_GAMMA_APPLY_TO_A8
-DQT_NO_KEYWORDS -DSK_ALLOW_STATIC_GLOBAL_INITIALIZERS=0 -DSK_SUPPORT_GPU=1
-DSK_FORCE_DISTANCE_FIELD_TEXT=0 -DSK_HAS_GIF_LIBRARY -DSK_HAS_JPEG_LIBRARY
-DSK_HAS_PNG_LIBRARY -DSK_HAS_WEBP_LIBRARY -DSK_TEST_QCMS -DSK_IS_BOT
-DSK_CODEC_DECODES_RAW -DSK_ARM_HAS_NEON -DSK_BUILD_FOR_ANDROID
-DSK_GAMMA_EXPONENT=1.4 -DSK_GAMMA_CONTRAST=0.0 -DSKIA_DLL
-DSKIA_IMPLEMENTATION=1 -DSK_SUPPORT_LEGACY_CLIPTOLAYERFLAG -DNDEBUG
-I../../../include/c -I../../../include/config -I../../../include/core
-I../../../include/pathops -I../../../include/ports -I../../../include/private
-I../../../include/utils -I../../../include/images -I../../../src/core
-I../../../src/sfnt -I../../../src/image -I../../../src/opts
-I../../../src/utils -I../../../include/gpu -I../../../src/gpu
-I../../../platform_tools/android/third_party/cpufeatures -fPIC -g
-fno-exceptions -fstrict-aliasing -Wall -Wextra -Winit-self -Wpointer-arith
-Wsign-compare -Wvla -Wno-unused-parameter -Werror -march=armv7-a -mthumb
-mfpu=neon -mfloat-abi=softfp -fuse-ld=gold -O2 -std=c++11 -fno-rtti
-fno-threadsafe-statics -Wnon-virtual-dtor  -c ../../../src/core/SkMatrix.cpp -o
obj/src/core/core.SkMatrix.o
../../../src/core/SkMatrix.cpp: In member function 'SkRect
SkMatrix::mapRectScaleTranslate(SkRect) const':
../../../src/core/SkMatrix.cpp:1120:1: error: unrecognizable insn:
 }
 ^
(insn 32 31 33 2 (set (reg:V4SF 115 [ D.83077 ])
        (unspec:V4SF [
                (mem/c:V4SF (reg/f:SI 104 virtual-incoming-args) [0 MEM[(const
__builtin_neon_sf[4] *)&src]+0 S16 A32])
            ] UNSPEC_VLD1))
/b/work/skia/platform_tools/android/toolchains/arm-r11c-14/lib/gcc/arm-linux-androideabi/4.9/include/arm_neon.h:8693
-1
     (nil))
../../../src/core/SkMatrix.cpp:1120:1: internal compiler error: in extract_insn,
at recog.c:2202
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://source.android.com/source/report-bugs.html> for instructions.
.

[msarett, 2016-07-11 12:38:13]

Hmm that's strange, I don't know.

https://codereview.chromium.org/2137853002/diff/1/src/core/SkMatrix.cpp
File src/core/SkMatrix.cpp (right):

https://codereview.chromium.org/2137853002/diff/1/src/core/SkMatrix.cpp#newco...
src/core/SkMatrix.cpp:1118: Sk4f(min[0], min[1], max[0],
max[1]).store(&dst.fLeft);
Why the change to (0, 1, 0, 1) here?

[reed1, 2016-07-11 12:50:58]

On 2016/07/11 12:38:13, msarett wrote:
> Hmm that's strange, I don't know.
> 
> https://codereview.chromium.org/2137853002/diff/1/src/core/SkMatrix.cpp
> File src/core/SkMatrix.cpp (right):
> 
>
https://codereview.chromium.org/2137853002/diff/1/src/core/SkMatrix.cpp#newco...
> src/core/SkMatrix.cpp:1118: Sk4f(min[0], min[1], max[0],
> max[1]).store(&dst.fLeft);
> Why the change to (0, 1, 0, 1) here?

When returning by value, the current ABI uses two 2-slot registers. My min and
max happen to each be correct in their top-2 slots, so writing it this way is ~2
fewer instructions (at least on INTEL).

[mtklein, 2016-07-11 13:17:55]

This also looks like a compiler bug to me.  I don't see anything unusual in your
code.

I notice there's a newer Android NDK we could be using.  It might be worth
trying this again after I land that update.  If you want to try locally, patch
https://codereview.chromium.org/2140603002

[mtklein, 2016-07-11 13:32:05]

https://codereview.chromium.org/2137853002/diff/1/src/core/SkMatrix.cpp
File src/core/SkMatrix.cpp (left):

https://codereview.chromium.org/2137853002/diff/1/src/core/SkMatrix.cpp#oldco...
src/core/SkMatrix.cpp:1116: // We can extract either pair [0,1] or [2,3] from
min and max and be correct, but on
If you do keep this change this comment becomes misleading.