[aarch64-port-dev ] defined(AMD64) and InterpreterRuntime::popframe_move_outgoing_args

[Andrew Dinn]

[resending this, slightly updated, to the aarch64-port-dev list]

InterpreterRuntime::popframe_move_outgoing_args is conditionally
declared in the shared code at interpreterRuntime.hpp:139 but only
conditionally

  #if defined(IA32) || defined(AMD64) || defined(ARM)
    // Popframe support (only needed on x86, AMD64 and ARM)
    static void popframe_move_outgoing_args(JavaThread* thread, void*
src_address, void* dest_address);
  #endif

Interestingly, we refer to it in the aarch64 code at
templateInterpreter_aarch64.cpp:1698:

  __ super_call_VM_leaf(CAST_FROM_FN_PTR(address,
InterpreterRuntime::popframe_move_outgoing_args), rthread, c_rarg1,
c_rarg2);

Now, when building for our small AArch64 simulator with BUILTIN_SIM it
turns out that defined(AMD64) is true. I believe this is needed to build
library code correctly. So, we get the declaration we need It also
appears to work on the fastmodel, [presumably because -DAMD64 is also
defined in the fastmodel build? This is a tad weird.

n.b. in interpreterRuntime.cpp the definition of
popframe_move_outgoing_args is guarded with the following

  #if defined(IA32) || defined(AMD64) || defined(ARM) ||
defined(TARGET_ARCH_aarch64)

In the C2 branch I have modified the declaration in
interpreterRuntime.hpp to include this extra disjunct.

I only spotted this because there are a few cases in the opto code where
declarations occur under the scope of

  #if defined(IA32) || defined(AMD64)

For example, in machnode.hpp we have

#if defined(IA32) || defined(AMD64)
  XMMRegister  as_XMMRegister(PhaseRegAlloc *ra_, const Node *node)
const {
    return ::as_XMMRegister(reg(ra_, node));
  }
  XMMRegister  as_XMMRegister(PhaseRegAlloc *ra_, const Node *node, int
idx)   const {
    return ::as_XMMRegister(reg(ra_, node, idx));
  }
#endif

We /don't/ want to see an XMM register in AArch64 code so we need to
avoid this. In the C" branch I have bracketed such occurrences with a
surrounding

  #ifdef BUILTIN_SIM
  #else
  . . .
  #endif

This makes the assumption that AMD64 will not be defined when building
for ARM's fastmodel. However, this assumption appears to be wrong which
means that we need to undefine AMD64 and deal with any consequences
before C2 will not compile correctly.

Alternatively, we could change the bracketing ifdef to be

  #ifdef TARGET_AARCH_aarch64
  #else
  . . .
  #endif

but I think that would be an incorrect fix.

regards,


Andrew Dinn
-----------