[GR-7625] [GR-9090] Fix JNI jfieldID and provide number of vector reg…

…ister arguments for C calls. PullRequest: graal/1229
maraist · Mar 30, 2018 · bc9b056 · bc9b056
2 parents 4277852 + 2bdeddf
commit bc9b056
Show file tree

Hide file tree

Showing 11 changed files with 96 additions and 48 deletions.
diff --git a/...oracle.svm.core.graal/src/com/oracle/svm/core/graal/code/amd64/SubstrateAMD64Backend.java b/...oracle.svm.core.graal/src/com/oracle/svm/core/graal/code/amd64/SubstrateAMD64Backend.java
@@ -29,6 +29,9 @@
 import static jdk.vm.ci.amd64.AMD64.rsp;
 import static jdk.vm.ci.amd64.AMD64.xmm0;
 import static jdk.vm.ci.code.ValueUtil.asRegister;
+import static jdk.vm.ci.code.ValueUtil.isRegister;
+
+import java.util.Collection;
 
 import org.graalvm.collections.EconomicSet;
 import org.graalvm.compiler.asm.Assembler;
@@ -92,6 +95,7 @@
 import org.graalvm.compiler.nodes.NodeView;
 import org.graalvm.compiler.nodes.SafepointNode;
 import org.graalvm.compiler.nodes.StructuredGraph;
+import org.graalvm.compiler.nodes.ValueNode;
 import org.graalvm.compiler.nodes.spi.NodeLIRBuilderTool;
 import org.graalvm.compiler.nodes.spi.NodeValueMap;
 import org.graalvm.compiler.options.OptionValues;
@@ -434,6 +438,27 @@ protected DebugInfoBuilder createDebugInfoBuilder(StructuredGraph graph, NodeVal
             return new SubstrateDebugInfoBuilder(nodeValueMap, graph.getDebug());
         }
 
+        @Override
+        public Value[] visitInvokeArguments(CallingConvention invokeCc, Collection<ValueNode> arguments) {
+            Value[] values = super.visitInvokeArguments(invokeCc, arguments);
+
+            SubstrateCallingConventionType type = (SubstrateCallingConventionType) ((SubstrateCallingConvention) invokeCc).getType();
+            if (type.nativeABI) {
+                // Native functions might have varargs, in which case we need to set %al to the
+                // number of XMM registers used for passing arguments
+                int xmmCount = 0;
+                for (Value v : values) {
+                    if (isRegister(v) && asRegister(v).getRegisterCategory().equals(AMD64.XMM)) {
+                        xmmCount++;
+                    }
+                }
+                assert xmmCount <= 8;
+                AllocatableValue xmmCountRegister = AMD64.rax.asValue(LIRKind.value(AMD64Kind.DWORD));
+                gen.emitMoveConstant(xmmCountRegister, JavaConstant.forInt(xmmCount));
+            }
+            return values;
+        }
+
         @Override
         protected void emitDirectCall(DirectCallTargetNode callTarget, Value result, Value[] parameters, Value[] temps, LIRFrameState callState) {
             ResolvedJavaMethod targetMethod = callTarget.targetMethod();
@@ -442,9 +467,13 @@ protected void emitDirectCall(DirectCallTargetNode callTarget, Value result, Val
 
         @Override
         protected void emitIndirectCall(IndirectCallTargetNode callTarget, Value result, Value[] parameters, Value[] temps, LIRFrameState callState) {
-            // The current register allocator cannot handle variables at call sites, need a fixed
-            // register.
-            AllocatableValue targetAddress = AMD64.rax.asValue(FrameAccess.getWordStamp().getLIRKind(getLIRGeneratorTool().getLIRKindTool()));
+            // The register allocator cannot handle variables at call sites, need a fixed register.
+            Register targetRegister = AMD64.rax;
+            if (((SubstrateCallingConventionType) callTarget.callType()).nativeABI) {
+                // Do not use RAX for C calls, it contains the number of XMM registers for varargs.
+                targetRegister = AMD64.r10;
+            }
+            AllocatableValue targetAddress = targetRegister.asValue(FrameAccess.getWordStamp().getLIRKind(getLIRGeneratorTool().getLIRKindTool()));
             gen.emitMove(targetAddress, operand(callTarget.computedAddress()));
             ResolvedJavaMethod targetMethod = callTarget.targetMethod();
             append(new AMD64Call.IndirectCallOp(targetMethod, result, parameters, temps, targetAddress, callState));

diff --git a/...svm.core.graal/src/com/oracle/svm/core/graal/code/amd64/SubstrateAMD64RegisterConfig.java b/...svm.core.graal/src/com/oracle/svm/core/graal/code/amd64/SubstrateAMD64RegisterConfig.java
@@ -248,9 +248,7 @@ public CallingConvention getCallingConvention(Type t, JavaType returnType, JavaT
 
         JavaKind returnKind = returnType == null ? JavaKind.Void : ObjectLayout.getCallSignatureKind(isEntryPoint, (ResolvedJavaType) returnType, metaAccess, target);
         AllocatableValue returnLocation = returnKind == JavaKind.Void ? Value.ILLEGAL : getReturnRegister(returnKind).asValue(valueKindFactory.getValueKind(returnKind.getStackKind()));
-        SubstrateCallingConvention callingConvention = new SubstrateCallingConvention(currentStackOffset, returnLocation, locations);
-        callingConvention.setArgumentStorageKinds(kinds);
-        return callingConvention;
+        return new SubstrateCallingConvention(type, kinds, currentStackOffset, returnLocation, locations);
     }
 
     @Override

diff --git a/...e.svm.core.graal/src/com/oracle/svm/core/graal/code/amd64/SubstrateCallingConvention.java b/...e.svm.core.graal/src/com/oracle/svm/core/graal/code/amd64/SubstrateCallingConvention.java
@@ -27,14 +27,17 @@
 import jdk.vm.ci.meta.JavaKind;
 
 public class SubstrateCallingConvention extends CallingConvention {
-    private JavaKind[] kinds;
+    private final Type type;
+    private final JavaKind[] kinds;
 
-    SubstrateCallingConvention(int stackSize, AllocatableValue returnLocation, AllocatableValue... argumentLocations) {
+    SubstrateCallingConvention(Type type, JavaKind[] kinds, int stackSize, AllocatableValue returnLocation, AllocatableValue... argumentLocations) {
         super(stackSize, returnLocation, argumentLocations);
+        this.type = type;
+        this.kinds = kinds;
     }
 
-    void setArgumentStorageKinds(JavaKind[] kinds) {
-        this.kinds = kinds;
+    public Type getType() {
+        return type;
     }
 
     public JavaKind[] getArgumentStorageKinds() {

diff --git a/...m.core.graal/src/com/oracle/svm/core/graal/code/amd64/SubstrateCallingConventionType.java b/...m.core.graal/src/com/oracle/svm/core/graal/code/amd64/SubstrateCallingConventionType.java
@@ -23,7 +23,6 @@
 package com.oracle.svm.core.graal.code.amd64;
 
 import jdk.vm.ci.code.CallingConvention;
-import jdk.vm.ci.code.CallingConvention.Type;
 
 public enum SubstrateCallingConventionType implements CallingConvention.Type {
     /**
@@ -52,8 +51,6 @@ public enum SubstrateCallingConventionType implements CallingConvention.Type {
 
     public final boolean nativeABI;
 
-    public static final Type[] VALUES = values();
-
     SubstrateCallingConventionType(boolean outgoing, boolean nativeABI) {
         this.outgoing = outgoing;
         this.nativeABI = nativeABI;

diff --git a/substratevm/src/com.oracle.svm.jni/src/com/oracle/svm/jni/JNIGeneratedMethodSupport.java b/substratevm/src/com.oracle.svm.jni/src/com/oracle/svm/jni/JNIGeneratedMethodSupport.java
@@ -29,12 +29,15 @@
 import org.graalvm.compiler.word.Word;
 import org.graalvm.nativeimage.c.type.CIntPointer;
 import org.graalvm.word.PointerBase;
+import org.graalvm.word.WordBase;
 
 import com.oracle.svm.core.StaticFieldsSupport;
 import com.oracle.svm.core.UnsafeAccess;
 import com.oracle.svm.core.config.ConfigurationValues;
+import com.oracle.svm.jni.access.JNIAccessibleField;
 import com.oracle.svm.jni.access.JNINativeLinkage;
 import com.oracle.svm.jni.nativeapi.JNIEnvironment;
+import com.oracle.svm.jni.nativeapi.JNIFieldId;
 import com.oracle.svm.jni.nativeapi.JNIObjectHandle;
 
 import jdk.vm.ci.meta.JavaKind;
@@ -72,6 +75,10 @@ static Object unboxHandle(JNIObjectHandle handle) {
         return JNIObjectHandles.getObject(handle);
     }
 
+    static WordBase getFieldOffsetFromId(JNIFieldId fieldId) {
+        return JNIAccessibleField.getOffsetFromId(fieldId);
+    }
+
     static byte[] getStaticPrimitiveFieldsArray() {
         return StaticFieldsSupport.getStaticPrimitiveFields();
     }

diff --git a/substratevm/src/com.oracle.svm.jni/src/com/oracle/svm/jni/access/JNIAccessFeature.java b/substratevm/src/com.oracle.svm.jni/src/com/oracle/svm/jni/access/JNIAccessFeature.java
@@ -267,7 +267,7 @@ private static void addField(Field reflField, DuringAnalysisAccessImpl access) {
                 FieldTypeFlow instanceFieldFlow = field.getDeclaringClass().getContextInsensitiveAnalysisObject().getInstanceFieldFlow(bigBang, field, true);
                 declaredTypeFlow.addUse(bigBang, instanceFieldFlow);
             }
-            return new JNIAccessibleField(jniClass, reflField.getName(), field.getModifiers());
+            return new JNIAccessibleField(jniClass, reflField.getName(), field.getJavaKind(), field.getModifiers());
         });
     }
 

diff --git a/substratevm/src/com.oracle.svm.jni/src/com/oracle/svm/jni/access/JNIAccessibleField.java b/substratevm/src/com.oracle.svm.jni/src/com/oracle/svm/jni/access/JNIAccessibleField.java
@@ -27,48 +27,64 @@
 import java.lang.reflect.Field;
 import java.lang.reflect.Modifier;
 
+import org.graalvm.compiler.word.Word;
+import org.graalvm.nativeimage.Platform.HOSTED_ONLY;
+import org.graalvm.nativeimage.Platforms;
+import org.graalvm.word.UnsignedWord;
+import org.graalvm.word.WordBase;
+
 import com.oracle.svm.core.StaticFieldsSupport;
 import com.oracle.svm.hosted.FeatureImpl.CompilationAccessImpl;
 import com.oracle.svm.hosted.meta.HostedField;
+import com.oracle.svm.jni.nativeapi.JNIFieldId;
+
+import jdk.vm.ci.meta.JavaKind;
 
 /**
  * Information on a class that can be looked up and accessed via JNI.
  */
 public final class JNIAccessibleField {
-    private final JNIAccessibleClass declaringClass;
-    private final String name;
-    private final int modifiers;
+    private static final UnsignedWord ID_STATIC_FLAG = Word.unsigned(-1L).unsignedShiftRight(1).add(1);
+    private static final UnsignedWord ID_OBJECT_FLAG = ID_STATIC_FLAG.unsignedShiftRight(1);
+    private static final UnsignedWord ID_OFFSET_MASK = ID_OBJECT_FLAG.subtract(1);
 
     /**
      * For instance fields, the offset of the field in an object of {@link #declaringClass}. For
      * static fields, depending on the field's type, the offset of the field in either
      * {@link StaticFieldsSupport#getStaticPrimitiveFields()} or
      * {@link StaticFieldsSupport#getStaticObjectFields()}.
      */
-    private int offset = -1;
+    public static WordBase getOffsetFromId(JNIFieldId id) {
+        return ((UnsignedWord) id).and(ID_OFFSET_MASK);
+    }
 
-    JNIAccessibleField(JNIAccessibleClass declaringClass, String name, int modifiers) {
+    private final JNIAccessibleClass declaringClass;
+    private final String name;
+    @Platforms(HOSTED_ONLY.class) private final UnsignedWord flags;
+    private UnsignedWord id = Word.zero();
+
+    JNIAccessibleField(JNIAccessibleClass declaringClass, String name, JavaKind kind, int modifiers) {
         this.declaringClass = declaringClass;
         this.name = name;
-        this.modifiers = modifiers;
-    }
 
-    public int getOffset() {
-        assert offset != -1;
-        return offset;
+        UnsignedWord bits = Modifier.isStatic(modifiers) ? ID_STATIC_FLAG : Word.zero();
+        bits = bits.or(kind.isObject() ? ID_OBJECT_FLAG : Word.zero());
+        this.flags = bits;
     }
 
-    public boolean isStatic() {
-        return Modifier.isStatic(modifiers);
+    public JNIFieldId getId() {
+        return (JNIFieldId) id;
     }
 
     void fillOffset(CompilationAccessImpl access) {
-        assert offset == -1;
+        assert id.equal(0);
         try {
             Field reflField = declaringClass.getClassObject().getDeclaredField(name);
             HostedField field = access.getMetaAccess().lookupJavaField(reflField);
             assert field.hasLocation();
-            offset = field.getLocation();
+            int offset = field.getLocation();
+            assert ID_OFFSET_MASK.and(offset).equal(offset);
+            this.id = flags.or(offset);
         } catch (NoSuchFieldException e) {
             throw new RuntimeException(e);
         }

diff --git a/...tratevm/src/com.oracle.svm.jni/src/com/oracle/svm/jni/access/JNIReflectionDictionary.java b/...tratevm/src/com.oracle.svm.jni/src/com/oracle/svm/jni/access/JNIReflectionDictionary.java
@@ -135,24 +135,17 @@ public JNIAccessibleField getField(Class<?> classObject, String name) {
         return (clazz != null) ? clazz.getField(name) : null;
     }
 
-    public JNIFieldId getFieldID(Class<?> clazz, String name, boolean isStatic) {
+    public JNIFieldId getFieldID(Class<?> clazz, String name) {
         JNIAccessibleField field = getField(clazz, name);
-        if (field != null && field.isStatic() == isStatic) {
-            return getFieldID(field);
-        }
-        return Word.nullPointer();
-    }
-
-    private static JNIFieldId getFieldID(JNIAccessibleField field) {
-        return WordFactory.pointer((long) field.getOffset());
+        return field != null ? field.getId() : Word.zero();
     }
 
-    public String getFieldNameByID(Class<?> classObject, JNIFieldId id, boolean isStatic) {
+    public String getFieldNameByID(Class<?> classObject, JNIFieldId id) {
         JNIAccessibleClass clazz = classesByClassObject.get(classObject);
         if (clazz != null) {
             for (Entry<String, JNIAccessibleField> entry : clazz.getFieldsByName().entrySet()) {
                 JNIAccessibleField field = entry.getValue();
-                if (isStatic == field.isStatic() && id.equal(getFieldID(field))) {
+                if (id.equal(field.getId())) {
                     return entry.getKey();
                 }
             }

diff --git a/substratevm/src/com.oracle.svm.jni/src/com/oracle/svm/jni/functions/JNIFunctions.java b/substratevm/src/com.oracle.svm.jni/src/com/oracle/svm/jni/functions/JNIFunctions.java
@@ -391,13 +391,13 @@ static JNIMethodId GetStaticMethodID(JNIEnvironment env, JNIObjectHandle hclazz,
     @CEntryPoint
     @CEntryPointOptions(prologue = JNIEnvironmentEnterPrologue.class, exceptionHandler = JNIExceptionHandlerReturnNullWord.class, publishAs = Publish.NotPublished, include = CEntryPointOptions.NotIncludedAutomatically.class)
     static JNIFieldId GetFieldID(JNIEnvironment env, JNIObjectHandle hclazz, CCharPointer cname, CCharPointer csig) {
-        return Support.getFieldID(hclazz, cname, csig, false);
+        return Support.getFieldID(hclazz, cname, csig);
     }
 
     @CEntryPoint
     @CEntryPointOptions(prologue = JNIEnvironmentEnterPrologue.class, exceptionHandler = JNIExceptionHandlerReturnNullWord.class, publishAs = Publish.NotPublished, include = CEntryPointOptions.NotIncludedAutomatically.class)
     static JNIFieldId GetStaticFieldID(JNIEnvironment env, JNIObjectHandle hclazz, CCharPointer cname, CCharPointer csig) {
-        return Support.getFieldID(hclazz, cname, csig, true);
+        return Support.getFieldID(hclazz, cname, csig);
     }
 
     /*
@@ -815,12 +815,12 @@ static int GetJavaVM(JNIEnvironment env, JNIJavaVMPointer vm) throws Throwable {
     @CEntryPoint
     @CEntryPointOptions(prologue = JNIEnvironmentEnterPrologue.class, exceptionHandler = JNIExceptionHandlerReturnNullWord.class, publishAs = Publish.NotPublished, include = CEntryPointOptions.NotIncludedAutomatically.class)
     static JNIFieldId FromReflectedField(JNIEnvironment env, JNIObjectHandle fieldHandle) {
-        JNIFieldId fieldId = Word.nullPointer();
+        JNIFieldId fieldId = Word.zero();
         if (JNIAccessFeature.singleton().haveJavaRuntimeReflectionSupport()) {
             Field obj = JNIObjectHandles.getObject(fieldHandle);
             if (obj != null) {
                 boolean isStatic = Modifier.isStatic(obj.getModifiers());
-                fieldId = JNIReflectionDictionary.singleton().getFieldID(obj.getDeclaringClass(), obj.getName(), isStatic);
+                fieldId = JNIReflectionDictionary.singleton().getFieldID(obj.getDeclaringClass(), obj.getName());
             }
         }
         return fieldId;
@@ -831,12 +831,12 @@ static JNIFieldId FromReflectedField(JNIEnvironment env, JNIObjectHandle fieldHa
      */
     @CEntryPoint
     @CEntryPointOptions(prologue = JNIEnvironmentEnterPrologue.class, exceptionHandler = JNIExceptionHandlerReturnNullHandle.class, publishAs = Publish.NotPublished, include = CEntryPointOptions.NotIncludedAutomatically.class)
-    static JNIObjectHandle ToReflectedField(JNIEnvironment env, JNIObjectHandle classHandle, JNIFieldId fieldId, boolean isStatic) {
+    static JNIObjectHandle ToReflectedField(JNIEnvironment env, JNIObjectHandle classHandle, JNIFieldId fieldId) {
         Field field = null;
         if (JNIAccessFeature.singleton().haveJavaRuntimeReflectionSupport()) {
             Class<?> clazz = JNIObjectHandles.getObject(classHandle);
             if (clazz != null) {
-                String name = JNIReflectionDictionary.singleton().getFieldNameByID(clazz, fieldId, isStatic);
+                String name = JNIReflectionDictionary.singleton().getFieldNameByID(clazz, fieldId);
                 if (name != null) {
                     try {
                         field = clazz.getDeclaredField(name);
@@ -1002,11 +1002,11 @@ static JNIMethodId getMethodID(JNIObjectHandle hclazz, CCharPointer cname, CChar
             return JNIReflectionDictionary.singleton().getMethodID(clazz, name, signature, isStatic);
         }
 
-        static JNIFieldId getFieldID(JNIObjectHandle hclazz, CCharPointer cname, CCharPointer csig, boolean isStatic) {
+        static JNIFieldId getFieldID(JNIObjectHandle hclazz, CCharPointer cname, CCharPointer csig) {
             // TODO: check signature
             Class<?> clazz = JNIObjectHandles.getObject(hclazz);
             String name = CTypeConversion.toJavaString(cname);
-            return JNIReflectionDictionary.singleton().getFieldID(clazz, name, isStatic);
+            return JNIReflectionDictionary.singleton().getFieldID(clazz, name);
         }
 
         static CShortPointer pinStringAndGetChars(JNIObjectHandle hstr, CCharPointer isCopy) {

diff --git a/substratevm/src/com.oracle.svm.jni/src/com/oracle/svm/jni/hosted/JNIFieldAccessorMethod.java b/substratevm/src/com.oracle.svm.jni/src/com/oracle/svm/jni/hosted/JNIFieldAccessorMethod.java
@@ -145,7 +145,8 @@ public StructuredGraph buildGraph(DebugContext debug, ResolvedJavaMethod method,
             ValueNode handle = arguments.get(1);
             object = kit.unboxHandle(handle);
         }
-        ValueNode offset = arguments.get(2);
+        ValueNode fieldId = arguments.get(2);
+        ValueNode offset = kit.getFieldOffsetFromId(fieldId);
         ValueNode returnValue;
         if (isSetter) {
             returnValue = null; // void

diff --git a/substratevm/src/com.oracle.svm.jni/src/com/oracle/svm/jni/hosted/JNIGraphKit.java b/substratevm/src/com.oracle.svm.jni/src/com/oracle/svm/jni/hosted/JNIGraphKit.java
@@ -87,6 +87,10 @@ public InvokeWithExceptionNode unboxHandle(ValueNode handle) {
         return createStaticInvoke("unboxHandle", handle);
     }
 
+    public InvokeWithExceptionNode getFieldOffsetFromId(ValueNode fieldId) {
+        return createStaticInvoke("getFieldOffsetFromId", fieldId);
+    }
+
     public InvokeWithExceptionNode getStaticPrimitiveFieldsArray() {
         return createStaticInvoke("getStaticPrimitiveFieldsArray");
     }