Make FlushICache NOP for Nvidia Denver 1.0 only

Make FlushICache NOP for Nvidia Denver 1.0 only

FlushICache should be NOP for Denver with part numbers 0x0, 0x1 and 0x2 only.
Instruction cache needs to flushed for future versions of denver.

Committed: https://crrev.com/434a291a0a05015d247c3604c27f88caaa181a51
Cr-Commit-Position: refs/heads/master@{#30262}

[Srikumar, 2015-08-19 00:38:04]

sbonda@nvidia.com changed reviewers:
+ bmeurer@chromium.org, rmcilroy@chromium.org

[Srikumar, 2015-08-19 02:55:32]

sbonda@nvidia.com changed reviewers:
+ arajp@nvidia.com

[arajp, 2015-08-19 04:04:31]

On 2015/08/19 02:55:32, Srikumar wrote:
> mailto:sbonda@nvidia.com changed reviewers:
> + mailto:arajp@nvidia.com

LGTM

[Benedikt Meurer, 2015-08-19 08:13:44]

The CQ bit was checked by bmeurer@chromium.org

[Benedikt Meurer, 2015-08-19 08:13:44]

lgtm

[commit-bot: I haz the power, 2015-08-19 08:13:53]

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

[Rodolph Perfetta (ARM), 2015-08-19 08:19:26]

rodolph.perfetta@arm.com changed reviewers:
+ rodolph.perfetta@arm.com

[Rodolph Perfetta (ARM), 2015-08-19 08:19:27]

https://codereview.chromium.org/1287173004/diff/1/src/arm/assembler-arm.cc
File src/arm/assembler-arm.cc (right):

https://codereview.chromium.org/1287173004/diff/1/src/arm/assembler-arm.cc#ne...
src/arm/assembler-arm.cc:129: cpu.variant() == base::CPU::NVIDIA_DENVER &&
same as arm64

https://codereview.chromium.org/1287173004/diff/1/src/arm64/assembler-arm64.cc
File src/arm64/assembler-arm64.cc (right):

https://codereview.chromium.org/1287173004/diff/1/src/arm64/assembler-arm64.c...
src/arm64/assembler-arm64.cc:55: cpu.variant() == base::CPU::NVIDIA_DENVER &&
Denver is a CPU part (like A57, A53 for ARM), so it looks like the checks are
switched: part should be Denver and variant should be less than 0, 1 or 2. no?
The initial code was wrong then.

[Benedikt Meurer, 2015-08-19 08:19:51]

The CQ bit was unchecked by bmeurer@chromium.org

[Srikumar, 2015-08-19 16:13:54]

sbonda@nvidia.com changed reviewers:
+ avanbrunt@nvidia.com

[Srikumar, 2015-08-19 16:20:02]

I have added alex from nvidia cpu team to comment on variant() and part()
details for denver family.

[avanbrunt, 2015-08-19 16:35:39]

https://codereview.chromium.org/1287173004/diff/1/src/arm64/assembler-arm64.cc
File src/arm64/assembler-arm64.cc (right):

https://codereview.chromium.org/1287173004/diff/1/src/arm64/assembler-arm64.c...
src/arm64/assembler-arm64.cc:55: cpu.variant() == base::CPU::NVIDIA_DENVER &&
On 2015/08/19 08:19:27, Rodolph Perfetta (ARM) wrote:
> Denver is a CPU part (like A57, A53 for ARM), so it looks like the checks are
> switched: part should be Denver and variant should be less than 0, 1 or 2. no?
> The initial code was wrong then.

Implementer == 'N' and and variant == 0x0 combined identify the Denver family.
The different implementations are distinguished by the primary part number. Part
number 0x000, 0x001, and 0x002 all have a coherent instruction cache.

[Srikumar, 2015-08-19 17:33:28]

https://codereview.chromium.org/1287173004/diff/1/src/arm/assembler-arm.cc
File src/arm/assembler-arm.cc (right):

https://codereview.chromium.org/1287173004/diff/1/src/arm/assembler-arm.cc#ne...
src/arm/assembler-arm.cc:129: cpu.variant() == base::CPU::NVIDIA_DENVER &&
On 2015/08/19 08:19:27, Rodolph Perfetta (ARM) wrote:
> same as arm64

As mentioned by Alex on the other comment, cpu.implementer() as Nvidia and
cpu.variant() 0x0 check make it unique for denver family.

[Rodolph Perfetta (ARM), 2015-08-19 17:45:30]

On 2015/08/19 16:35:39, avanbrunt wrote:
> https://codereview.chromium.org/1287173004/diff/1/src/arm64/assembler-arm64.cc
> File src/arm64/assembler-arm64.cc (right):
> 
>
https://codereview.chromium.org/1287173004/diff/1/src/arm64/assembler-arm64.c...
> src/arm64/assembler-arm64.cc:55: cpu.variant() == base::CPU::NVIDIA_DENVER &&
> On 2015/08/19 08:19:27, Rodolph Perfetta (ARM) wrote:
> > Denver is a CPU part (like A57, A53 for ARM), so it looks like the checks
are
> > switched: part should be Denver and variant should be less than 0, 1 or 2.
no?
> > The initial code was wrong then.
> 
> Implementer == 'N' and and variant == 0x0 combined identify the Denver family.
> The different implementations are distinguished by the primary part number.
Part
> number 0x000, 0x001, and 0x002 all have a coherent instruction cache.

I see. It is different on ARM (implementer and part define a CPU) so by force of
habit I assumed the code was wrong. Sorry for that.

[Srikumar, 2015-08-20 04:43:29]

The CQ bit was checked by sbonda@nvidia.com

[commit-bot: I haz the power, 2015-08-20 04:43:32]

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

[commit-bot: I haz the power, 2015-08-20 04:44:43]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-08-20 04:44:55]

Patchset 1 (id:??) landed as
https://crrev.com/434a291a0a05015d247c3604c27f88caaa181a51
Cr-Commit-Position: refs/heads/master@{#30262}

[Srikumar, 2015-08-20 04:45:23]

As all the comments are addressed, I ticked the commit option.

[Srikumar, 2015-08-24 15:13:23]

sbonda@nvidia.com changed reviewers:
- arajp@nvidia.com, avanbrunt@nvidia.com

[Srikumar, 2015-08-24 15:13:47]

Benedikt Meurer /  Rodolph Perfetta, 

This fix is very critical for our future bringup and will unblock our execution.
Hence, May i kindly request you or appropriate to integrate this change to
upcoming release(s) of chrome and webview M45 and M44 minor update (if anything
planned)?

[Srikumar, 2015-08-24 15:16:43]

On 2015/08/24 15:13:47, Srikumar wrote:
> Benedikt Meurer /  Rodolph Perfetta, 
> 
> This fix is very critical for our future bringup and will unblock our
execution.
> Hence, May i kindly request you or appropriate to integrate this change to
> upcoming release(s) of chrome and webview M45 and M44 minor update (if
anything
> planned)?

This change has no impact on non-nvidia products and in fact no performance
impact on Nexus 9 also.

[Srikumar, 2015-08-26 18:19:33]

Kind reminder.

[rmcilroy, 2015-08-27 08:41:49]

rmcilroy@chromium.org changed reviewers:
+ hablich@chromium.org

[rmcilroy, 2015-08-27 08:41:49]

+hablich for merge request (see comments above).

[Michael Hablich, 2015-08-27 08:47:25]

On 2015/08/27 08:41:49, rmcilroy wrote:
> +hablich for merge request (see comments above).
Can you please create an issue according to
https://code.google.com/p/v8-wiki/wiki/MergingAndPatching and highlight the
impact (what problem is it fixing)? Please CC me!

For M44 the ship has already sailed. If this fix is critical enough we can merge
it to M45 in a future patch.