[foreign-memaccess+abi] RFR: 8308293: A linker should expose the layouts it supports

[Jorn Vernee]

On Thu, 15 Jun 2023 16:05:52 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> src/java.base/share/classes/jdk/internal/foreign/abi/AbstractLinker.java line 142:
>> 
>>> 140:     @Override
>>> 141:     public Map<String, MemoryLayout> canonicalLayouts() {
>>> 142:         return CANONICAL_LAYOUTS_MAP;
>> 
>> I would prefer each linker to define it's own `static final` map, and implement canonicalLayouts() individually, rather than having a bunch of selection logic here in `AbstractLinker`. Since the mappings are ABI specific, and the linkers implement the ABI specific code, I think that is where this belongs.
>> 
>> Since most of the layouts are the same, I suggest adding a common factory method to SharedUtils with parameters for the layouts that change. (In practice we only use a single ABI per process, so we should only create the map once, and C2 should be able to fold away the virtual call).
>
> I'm not too sure on what this shared factories would look like. In the general case (e.g. fallback) it would have at least to specify the size of short/int/long/long long. Plus possibly, size_t. Basically everything besides float/double/char/bool. I'm not sure that would end up simpler than just having duplicated code with the layout creation?

For the fallback linker I agree, but for the other platforms, it seems that only the layout of `long` and `size_t` changes? I was thinking something like:


static Map<String, MemoryLayout> makeCanonicalLayouts(ValueLayout longLayout, ValueLayout sizeTLayout) {
    return Map.ofEntries(
            // specified canonical layouts
            Map.entry("bool", ValueLayout.JAVA_BOOLEAN),
            Map.entry("char", ValueLayout.JAVA_BYTE),
            Map.entry("short", ValueLayout.JAVA_SHORT),
            Map.entry("int", ValueLayout.JAVA_INT),
            Map.entry("float", ValueLayout.JAVA_FLOAT),
            Map.entry("long", longLayout),
            Map.entry("long long", ValueLayout.JAVA_LONG),
            Map.entry("double", ValueLayout.JAVA_DOUBLE),
            Map.entry("void*", ValueLayout.ADDRESS),
            Map.entry("size_t", sizeTLayout),
            // unspecified size-dependent layouts
            Map.entry("int8_t", ValueLayout.JAVA_BYTE),
            Map.entry("int16_t", ValueLayout.JAVA_SHORT),
            Map.entry("int32_t", ValueLayout.JAVA_INT),
            Map.entry("int64_t", ValueLayout.JAVA_LONG),
            // unspecified JNI layouts
            Map.entry("jboolean", ValueLayout.JAVA_BOOLEAN),
            Map.entry("jchar", ValueLayout.JAVA_CHAR),
            Map.entry("jbyte", ValueLayout.JAVA_BYTE),
            Map.entry("jshort", ValueLayout.JAVA_SHORT),
            Map.entry("jint", ValueLayout.JAVA_INT),
            Map.entry("jlong", ValueLayout.JAVA_LONG),
            Map.entry("jfloat", ValueLayout.JAVA_FLOAT),
            Map.entry("jdouble", ValueLayout.JAVA_DOUBLE)
    );
}


Then each linker can have:


private static final ValueLayout C_LONG = ...
private static final ValueLayout C_SIZE_T = ...
private static final Map<String, MemoryLayout> CANONICAL_LAYOUTS_MAP = SharedUtils.makeCanonicalLayouts(C_LONG, C_SIZE_T);

@Override
public Map<String, MemoryLayout> canonicalLayouts() {
    return CANONICAL_LAYOUTS_MAP;
 }

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

PR Review Comment: https://git.openjdk.org/panama-foreign/pull/839#discussion_r1231275738