Bug 138499 – Enable Cortex-A53-specific code paths by default if core is detected

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Akos Kiss]

This is a follow-up to https://bugs.webkit.org/show_bug.cgi?id=138315 . It would be good if the build system would detect that we are on a Cortex-A53 and enable specific code paths by default, with no need for --cmakeargs="-DWTF_CPU_ARM64_CORTEXA53=ON" command line arguments (which are easy to forget or misspell).

[Akos Kiss]

Created attachment 241168 [details]
Proposed patch.

[Csaba Osztrogonác]

Comment on attachment 241168 [details]
Proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=241168&action=review

> Source/cmake/OptionsCommon.cmake:36
> +    if (CPUINFO MATCHES "0xd03")

I would restrict matching "0xd03" only in "CPU part" line to avoid accidental matches.

And could you possibly add a comment to the ARM reference?
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0500f/BABFEABI.html

> Source/cmake/OptionsCommon.cmake:37
> +        set(WTF_CPU_ARM64_CORTEXA53_INITIALVALUE ON)

Why not set(WTF_CPU_ARM64_CORTEXA53 1) and add_definitions(-DWTF_CPU_ARM64_CORTEXA53=1) ?
In this case we wouldn't need WTF_CPU_ARM64_CORTEXA53_INITIALVALUE and the option.

[Akos Kiss]

Created attachment 241176 [details]
Proposed patch, v2

The updated patch now checks for 0xd03 only in the CPU part line, as requested. Also, comments are added. 

Moreover, the patch executes the check multiple times, once for each available core, because the /proc/cpuinfo API is somewhat strange on ARM64 and the result depends on the core the query is executed on. This might conceal the presence of an A53 core in a heterogeneous setup.

Option and _INITIALVALUE are used to allow explicit setting of WTF_CPU_ARM64_CORTEXA53 from command line, independently from the result of the detection.

[WebKit Commit Bot]

Attachment 241176 [details] did not pass style-queue:


ERROR: Source/cmake/OptionsCommon.cmake:46:  No space after "("  [whitespace/parentheses] [5]
ERROR: Source/cmake/OptionsCommon.cmake:46:  No space before ")"  [whitespace/parentheses] [5]
ERROR: Source/cmake/OptionsCommon.cmake:46:  One space between command "endforeach" and its parentheses, should be "endforeach ("  [whitespace/parentheses] [5]
Total errors found: 3 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Akos Kiss]

Created attachment 241178 [details]
Proposed patch, v3

One misplaced space. My bad. Sorry for the noise.

[Csaba Osztrogonác]

Comment on attachment 241178 [details]
Proposed patch, v3

r=me

[WebKit Commit Bot]

Comment on attachment 241178 [details]
Proposed patch, v3

Clearing flags on attachment: 241178

Committed r175804: <http://trac.webkit.org/changeset/175804>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.