runtime: arm64: add cortex-a53.a57 to the known CPUs list

rovo89 / android_art

Android ART with modifications for the Xposed framework.

Other

376 stars 212 forks source link

runtime: arm64: add cortex-a53.a57 to the known CPUs list #44

Closed TheCrazyLex closed 8 years ago

TheCrazyLex commented 8 years ago

We may want to tune for the big.LITTLE combiation consisting of cortex-a53 and cortex-a57 cores
As cortex-a53 cores are present, we are applying the needed arm errata fixes
If we set this combination as TARGET_CPU and this patch is not present, ART will completely fail dexopt at runtime

Change-Id: I3b6a58c22706c2b272810833895c29768ce76081 Signed-off-by: Alex Naidis alex.naidis@linux.com

TheCrazyLex commented 8 years ago

This is currently needed for two ROMs which propose to implement this soon: PA.
As said in the commit message, we would fail dexopt without this patch, when xposed is installed, so this is kind of a blocker.

It would be really good, if you could merge this and make something like a "hotfix release" for arm64. @rovo89

Thank you!

Alex

rovo89 commented 8 years ago

I can't find that change in AOSP: https://android.googlesource.com/platform/art/+/refs/heads/master/runtime/arch/arm64/instruction_set_features_arm64.cc Could you please point me to the relevant discussions?

TheCrazyLex commented 8 years ago

Hey @rovo89, this change is not in AOSP, the named custom roms would like this variant for improved tuning. We need this for ART to recognize and accept that as otherwise Dexopt fails, resulting in major lags and other issues.

Alex

rovo89 commented 8 years ago

Hm, but where did you get this commit from? I'm always a bit pessimistic about "performance tuning" cherry-picks that I don't understand.

The commit message also mentions that this avoids issues when TARGET_CPU is set to cortex-a53.a57 (which someone might do in order to add settings specific to that CPU combination). Are you planning to do that?

TheCrazyLex commented 8 years ago

Hey @rovo89 , I didn't get the commit from anywhere, I authored it by myself. It is good to be sceptical to performance tuning stuff, however this is not done here in ART, I added specific tuning instructions in bionic + build. This is just needed for the cpu to be recognized. Basically we could call this "cpu123" , if I adjust the names of the tuning instructions it would work. So this is just a identifier needed.

Alex

rovo89 commented 8 years ago

OK, good to know that you actually know what you're doing. :) My impression is that too many ROM developers merge everything that sounds like a performance improvement and don't understand the dependencies (e.g. on the bionic changes you mentioned).

Is this related (or maybe even a prerequisite)? https://android-review.googlesource.com/#/c/188172/ CyanogenMod merged it a while ago. I think I'll go through their cherry-picks first and select the relevant ones, then merge your commit.

rovo89 commented 8 years ago

Merged with 3417f07d4c1c48cc6621cab79e43d06bff42be2c, included in Xposed v85.

TheCrazyLex commented 8 years ago

Thank you @rovo89