eclipse-openj9 / openj9

Eclipse OpenJ9: A Java Virtual Machine for OpenJDK that's optimized for small footprint, fast start-up, and high throughput. Builds on Eclipse OMR (https://github.com/eclipse/omr) and combines with the Extensions for OpenJDK for OpenJ9 repo.
Other
3.28k stars 720 forks source link

[FFI] Handling the case of multi-dimensional arrays in preparing the ffi_type in downcall #20028

Open ChengJin01 opened 2 months ago

ChengJin01 commented 2 months ago

The issue is followed by the discussion starting from https://github.com/eclipse-openj9/openj9/pull/19988#discussion_r1723758869 in which we don't directly support a nested 2-D array (never mentioned/covered in the Spec/Jtreg test suite given only structs/unions are treated as valid arguments passed to downcall in which arrays are elements within them). That being said, the code there might be refactored a little bit if we have to cover this special case.

e.g. the output generated by jextract as follows:

/**
 * {@snippet lang=c :
 * struct S {
 *     int a[3][2];
 * }
 * }
 */
public class S {

    S() {
        // Should not be called directly
    }

    private static final GroupLayout $LAYOUT = MemoryLayout.structLayout(
        MemoryLayout.sequenceLayout(3, MemoryLayout.sequenceLayout(2, array_h.C_INT)).withName("a")
    ).withName("S");

FYI: @tajila, @pshipton, @keithc-ca.

ChengJin01 commented 2 months ago

One way is to directly modify the existing code in native to handle this case which might need new methods or code refactoring to accommodate the situation; the other way (which I prefer to assuming it works in this way) is to slightly change the code in java (the preprocessing of layout string in LayoutStrPreprocessor.java) to directly convert the 2-D array to a 1-D array (e.g. int a[3][2] --> 3:2:I ---> 6:I) without updating any code in native.

keithc-ca commented 2 months ago

convert the 2-D array to a 1-D array

I hadn't thought of that, but I like the idea.

ChengJin01 commented 2 months ago

With my fix at https://github.com/ChengJin01/openj9/commit/3800215b4d448ae4b7ce2142cb38509ee516d672, it works good for the conversion as expected, which can be refined if anything. e.g. in the test, it converts int[3][3] to [9:I] for the struct layout string.