> 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
