[code-reflection] RFR: Float4 arrayView support

[Juan Fumero]

On Mon, 27 Oct 2025 23:56:01 GMT, Ruby Chen <duke at openjdk.org> wrote:

> Preliminary `Float4` support for arrayViews. Currently only supported for `F32ArrayPadded` buffers, which can be accessed like a `Float4[]` as shown below:
> 
> Float4[] arr = buffer.float4ArrayView(); // for a F32ArrayPadded buffer
> 
> At the moment, to use an element in the array, the `Float4` must be loaded into a separate variable first, i.e.
> 
> Float4 a = arr[index * 4];
> 
> A value can be stored into the array through the following syntax:
> 
> arr[index * 4] = ...

This is great work. I have a few comments/questions. 

All test passing also for my setup (OpenCL backend).

hat/backends/ffi/opencl/src/main/java/hat/backend/ffi/OpenCLHATKernelBuilder.java line 89:

> 87:                 .oparen();
> 88:         // if the value to be stored is an operation, recurse on the operation
> 89:         if (hatVectorStoreView.operands().get(1) instanceof Op.Result r && r.op() instanceof HATVectorBinaryOp) {

Shouldn't this be automatically visited when `HATVectorBinaryOp` is found? Do you have the use case in which this is needed?  The reason I am asking is that this also works with the prev. proposal for single Float4 loads./stores . So I wonder which pattern triggers the new check.

hat/core/src/main/java/hat/phases/HATDialectifyArrayViewPhase.java line 99:

> 97:             switch (op) {
> 98:                 case JavaOp.InvokeOp iop -> {
> 99:                     if (isVectorOperation(iop)) {

This HAT Phase is invoked after all vector operations have been inserted in the new code-model. So I wonder if this is actually triggered. My take is that, at this point in the compilation pipeline, all VectoLoads, Stores and VSelect are already in place. Is this correct?

hat/core/src/main/java/hat/phases/HATDialectifyArrayViewPhase.java line 165:

> 163:                 }
> 164:                 // handles only 1D and 2D arrays
> 165:                 case JavaOp.ArrayAccessOp.ArrayLoadOp alop -> {

My take is that this phase is hitting this `case` for all the actual array view. If the prev. block still insert the dialect ops for vectors, we could substitute the prev.. pipeline of hat phases with this one.

hat/core/src/main/java/hat/phases/HATDialectifyArrayViewPhase.java line 249:

> 247:                                     4,
> 248:                                     HATVectorViewOp.VectorType.FLOAT4,
> 249:                                     false, //TODO: fix

This `flag` was to indentify if the store is set to a private or shared array. In this case, we don't have a pointer. 

Using this search will probably work:


    private boolean findIsSharedOrPrivateSpace(CoreOp.VarAccessOp.VarLoadOp varLoadOp) {
        return findIsSharedOrPrivateSpace(varLoadOp.operands().get(0));
    }

    private boolean findIsSharedOrPrivateSpace(Value v) {
        if (v instanceof Op.Result r && r.op() instanceof CoreOp.VarAccessOp.VarLoadOp varLoadOp) {
            return findIsSharedOrPrivateSpace(varLoadOp);
        } else  if (v instanceof CoreOp.Result r && (r.op() instanceof HATLocalVarOp || r.op() instanceof HATPrivateVarOp)) {
            return true;
        }else{
            return false;
        }
    }


I think in the future we can add a new Op for expressing global array accesses too (from parameters), so we can simplify how we handle different levels of memory within the code gen.

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

PR Review: https://git.openjdk.org/babylon/pull/646#pullrequestreview-3386912390
PR Review Comment: https://git.openjdk.org/babylon/pull/646#discussion_r2468120990
PR Review Comment: https://git.openjdk.org/babylon/pull/646#discussion_r2468126202
PR Review Comment: https://git.openjdk.org/babylon/pull/646#discussion_r2468131031
PR Review Comment: https://git.openjdk.org/babylon/pull/646#discussion_r2468143307