RFR: 8302850: Implement C1 clone intrinsic that reuses arraycopy code for primitive arrays [v6]

[Galder Zamarreño]

On Fri, 15 Mar 2024 18:56:46 GMT, Dean Long <dlong at openjdk.org> wrote:

> OK, I missed the is_intrinsic_supported change. The platform-specific changes should probably have a comment saying they are for clone support.

That's fine for me, but the platform-specific changes are a bit spread around. Anywhere in particular you'd want the comment(s) to go in? Or shall the comments be added in all platform specific changes?

> Also, I was hoping there was a way to minimize platform-specific changes, maybe by handing the force_reexecute inheritance in state_for()....

I'm not sure exactly what you mean, but let me try to explain the `x->state_before()` bit. So I guess you're referring to this piece of logic:


  CodeEmitInfo* info = nullptr;
  if (x->state_before() != nullptr && x->state_before()->force_reexecute()) {
    info = state_for(x, x->state_before());
    info->set_force_reexecute();
  } else {
    info = state_for(x, x->state());
  }


This code was added to deal with SEGV failures coming out of `compiler/interpreter/Test6833129.clone_and_verify`, which is configure to stress deoptimizations. In `append_alloc_array_copy`, I construct `NewTypeArray` and the array_copy `Intrinsic` both with `state_before` so that if any deoptimizations happen at either stage, re-execution happen from the point before clone was called.

I guess I can move the code up to `LIRGenerator::state_for` and if the intrinsic is either array copy or new type array, apply that logic, is that what you are after?

> ... putting the state in x->state() instead of x->state_before()

Hmmm, you mean this instead? 


  CodeEmitInfo* info = nullptr;
  if (x->state_before() != nullptr && x->state_before()->force_reexecute()) {
    info = state_for(x, x->state());
    info->set_force_reexecute();
  } else {
    info = state_for(x, x->state());
  }


Granted that it could be tidied but not sure I understand how state can work here. Before this change both new type array and array copy called `CodeEmitInfo* info = state_for(x, x->state());` which didn't seem to be enough to be able to go back to re-executing bytecode before clone was called. It seemed to leave things in a half-done state when deoptimizations happened while clone's intrinsic was doing either a new type array or array copy.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/17667#issuecomment-2009168412