1516915 - GetBuildConfiguration in TestingFunctions.cpp should support arm7 detection.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P2)

Description

[Bob Clary [:bc] (inactive)]

GetBuildConfiguration in https://searchfox.org/mozilla-central/source/js/src/builtin/TestingFunctions.cpp#123 supports detecting 'arm64' but not 'arm7' which leads to the need to kludge the testing for 'arm7'.

If 'arm7' was available along with 'arm64' instead of using 

skip-if: !getBuildConfiguration()['x86'] && !getBuildConfiguration()['x64']

we could use

skip-if: getBuildConfiguration()['arm7'] || getBuildConfiguration()['arm64']

Bonus points if 'arm' was supported which would be true for either 'arm7' or 'arm64'.

Comment 1

[Lars T Hansen [:lth]]

Having an alias for two different architectures sounds like a bit of a footgun to me.  In what situations do you see a use for this?

Comment 2

[Lars T Hansen [:lth]]

Attachment: bug1516915-recognize-arm-in-build-configuration.patch

This just follows the existing pattern we use for ARM64: if "arm" is set the JIT generates code for ARM; if "arm-simulator" is not set then we're additionally running on real hardware.

Comment 3

[Bob Clary [:bc] (inactive)]

arm meaning just arm-32 would be sufficient for me. As for the use-case of arm7, arm64 and arm as an alias for either it would just simplify the skip-if conditions for tests which should be skipped for either and is not a requirement.

Comment 4

[Jan de Mooij [:jandem]]

Comment on attachment 9033924 [details] [diff] [review]
bug1516915-recognize-arm-in-build-configuration.patch

Review of attachment 9033924 [details] [diff] [review]:
-----------------------------------------------------------------

I assume you want to use this here and in other files?

https://searchfox.org/mozilla-central/source/js/src/jit-test/tests/wasm/spec/float_memory.wast.js#1

Comment 5

[Nicolas B. Pierron [:nbp]]

Thanks for the patch!

(In reply to Bob Clary [:bc:] from comment #0)
> skip-if: !getBuildConfiguration()['x86'] && !getBuildConfiguration()['x64']

I will suggest that we stay away from such negations in skip-if conditions as they are not easily grep-able.

Comment 6

[Bob Clary [:bc] (inactive)]

I plan to replace those with the arm-32 specific condition when it is available.

Comment 7

[Bob Clary [:bc] (inactive)]

Attachment: bug-1516915-skip-if-arm.patch

https://treeherder.mozilla.org/#/jobs?repo=try&tier=1%2C2%2C3&group_state=expanded&revision=820d5867522508b5ea0f4817e4c4fe4a7a7b952a&selectedJob=219736872

lth: If this passes review, would you please land it with your patch?

Comment 8

[Lars T Hansen [:lth]]

Comment on attachment 9034124 [details] [diff] [review]
bug-1516915-skip-if-arm.patch

Review of attachment 9034124 [details] [diff] [review]:
-----------------------------------------------------------------

Sure thing.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f391e2edd0f9
Add 'arm' (ie ARM-32 codegen) to the build configuration object. r=jandem
https://hg.mozilla.org/integration/mozilla-inbound/rev/44ac43e4e56f
Use new 'arm' property of the build configuration object in jit-test skip-if conditions, r=lth

Comment 10

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/f391e2edd0f9
https://hg.mozilla.org/mozilla-central/rev/44ac43e4e56f