> On 31 Oct 2019, at 05:30, David Holmes <david.hol...@oracle.com> wrote:
>
> Hi Doug,
>
> On 30/10/2019 10:06 pm, Doug Simon wrote:
>>> On 30 Oct 2019, at 05:28, David Holmes <david.hol...@oracle.com
>>> <mailto:david.hol...@oracle.com>> wrote:
>>>
>>> Hi Doug,
>>>
>>> Your proposed patch wasn't quite right. I made some adjustments but I'm
>>> still having issues with the test,
>>> HotSpotMethodSubstitutionTest.testThreadSubstitutions, as I don't see how
>>> to make the test execution conditional on the VM config.
>> Like this:
>
> Thanks for that! One alteration below ...
>
>> diff --git
>> a/compiler/src/org.graalvm.compiler.hotspot.test/src/org/graalvm/compiler/hotspot/test/HotSpotMethodSubstitutionTest.java
>>
>> b/compiler/src/org.graalvm.compiler.hotspot.test/src/org/graalvm/compiler/hotspot/test/HotSpotMethodSubstitutionTest.java
>> index 96e7d979d36..3c928b86961 100644
>> ---
>> a/compiler/src/org.graalvm.compiler.hotspot.test/src/org/graalvm/compiler/hotspot/test/HotSpotMethodSubstitutionTest.java
>> +++
>> b/compiler/src/org.graalvm.compiler.hotspot.test/src/org/graalvm/compiler/hotspot/test/HotSpotMethodSubstitutionTest.java
>> @@ -28,12 +28,13 @@ import java.lang.invoke.ConstantCallSite;
>> import java.lang.invoke.MethodHandles;
>> import java.lang.invoke.MethodType;
>> -import org.graalvm.compiler.nodes.IfNode;
>> -import org.junit.Test;
>> -
>> import org.graalvm.compiler.api.directives.GraalDirectives;
>> import org.graalvm.compiler.api.replacements.MethodSubstitution;
>> +import org.graalvm.compiler.hotspot.GraalHotSpotVMConfig;
>> +import org.graalvm.compiler.hotspot.HotSpotBackend;
>> +import org.graalvm.compiler.nodes.IfNode;
>> import org.graalvm.compiler.replacements.test.MethodSubstitutionTest;
>> +import org.junit.Test;
>> /**
>> * Tests HotSpot specific {@link MethodSubstitution}s.
>> @@ -133,13 +134,18 @@ public class HotSpotMethodSubstitutionTest extends
>> MethodSubstitutionTest {
>> @Test
>> public void testThreadSubstitutions() {
>> + GraalHotSpotVMConfig config = ((HotSpotBackend)
>> getBackend()).getRuntime().getVMConfig();
>> testGraph("currentThread");
>> - assertInGraph(testGraph("threadIsInterrupted", "isInterrupted",
>> true), IfNode.class);
>> - assertInGraph(testGraph("threadInterrupted", "isInterrupted",
>> true), IfNode.class);
>> + if (config.osThreadOffset != Integer.MAX_VALUE) {
>
> s/osThreadOffset/osThreadInterruptedOffset/
Good catch. Sorry about that.
>
> This change removes the interrupted field from osThread but not osThread
> itself. Though as osThread is only used to then access the interrupted field,
> I could both the same. Here's updated webrev showing that:
>
> http://cr.openjdk.java.net/~dholmes/8229516/webrev.v2/
> <http://cr.openjdk.java.net/~dholmes/8229516/webrev.v2/>
Looks good.
-Doug
