Skip to content

Commit

Permalink
[GR-15808] Mitigate against overflowing the max size limit for a HotS…
Browse files Browse the repository at this point in the history
…pot OopMap.

PullRequest: graal/3642
  • Loading branch information
dougxc committed May 24, 2019
2 parents de7a88a + 28ae51c commit 0fda852
Show file tree
Hide file tree
Showing 24 changed files with 325 additions and 231 deletions.
13 changes: 12 additions & 1 deletion compiler/mx.compiler/mx_compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,18 @@ def compiler_gate_benchmark_runner(tasks, extraVMarguments=None, prefix=''):
# ensure benchmark counters still work
if mx.get_arch() != 'aarch64': # GR-8364 Exclude benchmark counters on AArch64
with Task(prefix + 'DaCapo_pmd:BenchmarkCounters', tasks, tags=GraalTags.test) as t:
if t: _gate_dacapo('pmd', 1, _remove_empty_entries(extraVMarguments) + ['-XX:+UseJVMCICompiler', '-Dgraal.LIRProfileMoves=true', '-Dgraal.GenericDynamicCounters=true', '-Dgraal.TimedDynamicCounters=1000', '-XX:JVMCICounterSize=10'])
if t:
fd, logFile = tempfile.mkstemp()
os.close(fd) # Don't leak file descriptors
try:
_gate_dacapo('pmd', 1, _remove_empty_entries(extraVMarguments) + ['-Dgraal.LogFile=' + logFile, '-XX:+UseJVMCICompiler', '-Dgraal.LIRProfileMoves=true', '-Dgraal.GenericDynamicCounters=true', '-Dgraal.TimedDynamicCounters=1000', '-XX:JVMCICounterSize=10'])
with open(logFile) as fp:
haystack = fp.read()
needle = 'MoveOperations (dynamic counters)'
if needle not in haystack:
mx.abort('Expected to see "' + needle + '" in output:\n' + haystack)
finally:
os.remove(logFile)

# ensure -Xcomp still works
with Task(prefix + 'XCompMode:product', tasks, tags=GraalTags.test) as t:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ private LIRKind(PlatformKind platformKind, int referenceMask, int referenceCompr
this.referenceCompressionMask = referenceCompressionMask;
this.derivedReferenceBase = derivedReferenceBase;

assert this.referenceCompressionMask == 0 || this.referenceMask == this.referenceCompressionMask : "mixing compressed and uncompressed references is untested";
assert this.referenceCompressionMask == 0 || this.referenceMask == this.referenceCompressionMask : "mixing compressed and uncompressed references is unsupported";
assert derivedReferenceBase == null || !derivedReferenceBase.getValueKind(LIRKind.class).isDerivedReference() : "derived reference can't have another derived reference as base";
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@

import java.util.ArrayList;
import java.util.Arrays;
import java.util.BitSet;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -1008,16 +1007,12 @@ public LIRInstruction zapArgumentSpace() {
}

@Override
public VirtualStackSlot allocateStackSlots(int slots, BitSet objects, List<VirtualStackSlot> outObjectStackSlots) {
public VirtualStackSlot allocateStackSlots(int slots) {
builder.positionAtStart();
LLVMValueRef alloca = builder.buildPtrToInt(builder.buildArrayAlloca(slots), builder.longType());
builder.positionAtEnd(getBlockEnd(currentBlock));

LLVMStackSlot stackSlot = new LLVMStackSlot(alloca);
if (outObjectStackSlots != null) {
outObjectStackSlots.add(stackSlot);
}
return stackSlot;
return new LLVMStackSlot(alloca);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,11 @@ public static LIRGenerationResult emitLIR(Backend backend, StructuredGraph graph
}

@SuppressWarnings("try")
private static LIRGenerationResult emitLIR0(Backend backend, StructuredGraph graph, Object stub, RegisterConfig registerConfig, LIRSuites lirSuites,
private static LIRGenerationResult emitLIR0(Backend backend,
StructuredGraph graph,
Object stub,
RegisterConfig registerConfig,
LIRSuites lirSuites,
String[] allocationRestrictedTo) {
DebugContext debug = graph.getDebug();
try (DebugContext.Scope ds = debug.scope("EmitLIR"); DebugCloseable a = EmitLIR.start(debug)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,20 @@
public interface LIRGenerationProvider {
LIRGeneratorTool newLIRGenerator(LIRGenerationResult lirGenRes);

LIRGenerationResult newLIRGenerationResult(CompilationIdentifier compilationId, LIR lir, RegisterConfig registerConfig, StructuredGraph graph,
LIRGenerationResult newLIRGenerationResult(CompilationIdentifier compilationId,
LIR lir,
RegisterConfig registerConfig,
StructuredGraph graph,
Object stub);

NodeLIRBuilderTool newNodeLIRBuilder(StructuredGraph graph, LIRGeneratorTool lirGen);

/**
* Creates the object used to fill in the details of a given compilation result.
*/
CompilationResultBuilder newCompilationResultBuilder(LIRGenerationResult lirGenResult, FrameMap frameMap, CompilationResult compilationResult,
CompilationResultBuilder newCompilationResultBuilder(LIRGenerationResult lirGenResult,
FrameMap frameMap,
CompilationResult compilationResult,
CompilationResultBuilderFactory factory);

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ public void close() {

/**
* The {@link PrintStream} to which all non-suppressed output from {@link TTY} is written.
* Substituted by {@code com.oracle.svm.graal.Target_org_graalvm_compiler_debug_TTY}.
*/
public static final PrintStream out;
static {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
/*
* Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation. Oracle designates this
* particular file as subject to the "Classpath" exception as provided
* by Oracle in the LICENSE file that accompanied this code.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
package org.graalvm.compiler.hotspot.lir.test;

import java.util.ArrayList;
import java.util.List;

import org.graalvm.compiler.core.common.LIRKind;
import org.graalvm.compiler.hotspot.HotSpotBackend;
import org.graalvm.compiler.lir.VirtualStackSlot;
import org.graalvm.compiler.lir.framemap.FrameMapBuilder;
import org.graalvm.compiler.lir.gen.LIRGeneratorTool;
import org.graalvm.compiler.lir.jtt.LIRTest;
import org.graalvm.compiler.lir.jtt.LIRTestSpecification;
import org.graalvm.compiler.lir.stackslotalloc.LSStackSlotAllocator;
import org.graalvm.compiler.nodes.SafepointNode;
import org.graalvm.compiler.nodes.graphbuilderconf.GraphBuilderConfiguration;
import org.graalvm.compiler.nodes.graphbuilderconf.GraphBuilderContext;
import org.graalvm.compiler.nodes.graphbuilderconf.InvocationPlugin;
import org.junit.Assume;
import org.junit.Test;

import jdk.vm.ci.meta.AllocatableValue;
import jdk.vm.ci.meta.JavaConstant;
import jdk.vm.ci.meta.ResolvedJavaMethod;

/**
* Tests the mitigation against overflowing the max size limit for a HotSpot OopMap. The mitigation
* works by {@link LSStackSlotAllocator} placing reference typed stack slots at lower offsets.
*/
public class MitigateExceedingMaxOopMapStackOffsetTest extends LIRTest {

/**
* Allocate stacks slots and initializes those at an odd index with a reference constant and
* those at an even index with a primitive constant.
*/
private static class WriteStackValues extends LIRTestSpecification {
private final JavaConstant objectConstant;
private final JavaConstant primitiveConstant;

WriteStackValues(JavaConstant objectConstant, JavaConstant primitiveConstant) {
this.objectConstant = objectConstant;
this.primitiveConstant = primitiveConstant;
}

@Override
public void generate(LIRGeneratorTool gen) {
FrameMapBuilder frameMapBuilder = gen.getResult().getFrameMapBuilder();
LIRKind objectLirKind = LIRKind.reference(gen.target().arch.getPlatformKind(objectConstant.getJavaKind()));
LIRKind primitiveLirKind = LIRKind.value(gen.target().arch.getPlatformKind(primitiveConstant.getJavaKind()));

int numSlots = numPrimitiveSlots + numReferenceSlots;
List<AllocatableValue> slotList = new ArrayList<>(numSlots);
// Place reference slots at top and bottom of virtual frame
// with primitive slots in the middle. This tests that slot
// partitioning works.
for (int i = 0; i < numReferenceSlots / 2; i++) {
AllocatableValue src = gen.emitLoadConstant(objectLirKind, objectConstant);
VirtualStackSlot slot = frameMapBuilder.allocateSpillSlot(objectLirKind);
slotList.add(slot);
gen.emitMove(slot, src);
}
for (int i = 0; i < numPrimitiveSlots; i++) {
AllocatableValue src = gen.emitLoadConstant(objectLirKind, primitiveConstant);
VirtualStackSlot slot = frameMapBuilder.allocateSpillSlot(primitiveLirKind);
slotList.add(slot);
gen.emitMove(slot, src);
}
for (int i = numReferenceSlots / 2; i < numReferenceSlots; i++) {
AllocatableValue src = gen.emitLoadConstant(objectLirKind, objectConstant);
VirtualStackSlot slot = frameMapBuilder.allocateSpillSlot(objectLirKind);
slotList.add(slot);
gen.emitMove(slot, src);
}
slots = slotList.toArray(new AllocatableValue[slotList.size()]);
}
}

/**
* Read stacks slots and move their content into a blackhole.
*/
private static class ReadStackValues extends LIRTestSpecification {

ReadStackValues() {
}

@Override
public void generate(LIRGeneratorTool gen) {
for (int i = 0; i < slots.length; i++) {
gen.emitBlackhole(gen.emitMove(slots[i]));
}
}
}

@Override
protected GraphBuilderConfiguration editGraphBuilderConfiguration(GraphBuilderConfiguration conf) {
InvocationPlugin safepointPlugin = new InvocationPlugin() {
@Override
public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Receiver receiver) {
b.add(new SafepointNode());
return true;
}
};
conf.getPlugins().getInvocationPlugins().register(safepointPlugin, getClass(), "safepoint");
return super.editGraphBuilderConfiguration(conf);
}

/*
* Safepoint Snippet
*/
private static void safepoint() {
}

private static int numPrimitiveSlots;
private static int numReferenceSlots;
private static AllocatableValue[] slots;

private static final LIRTestSpecification readStackValues = new ReadStackValues();

@SuppressWarnings("unused")
@LIRIntrinsic
public static void instrinsic(LIRTestSpecification spec) {
}

private static final LIRTestSpecification writeStackValues = new WriteStackValues(JavaConstant.NULL_POINTER, JavaConstant.LONG_0);

public void testStackObjects() {
instrinsic(writeStackValues);
safepoint();
instrinsic(readStackValues);
}

@Test
public void runStackObjects() throws Throwable {
int max = ((HotSpotBackend) getBackend()).getRuntime().getVMConfig().maxOopMapStackOffset;
Assume.assumeFalse("no limit on oop map size", max == Integer.MAX_VALUE);
numPrimitiveSlots = (max / 8) * 2;
numReferenceSlots = (max / 8) - 100; // Should be enough margin for all platforms
runTest("testStackObjects");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
import org.graalvm.compiler.options.Option;
import org.graalvm.compiler.options.OptionKey;
import org.graalvm.compiler.options.OptionType;
import org.graalvm.compiler.options.OptionValues;
import org.graalvm.compiler.serviceprovider.GraalServices;
import org.graalvm.compiler.serviceprovider.ServiceProvider;

Expand All @@ -60,7 +59,7 @@ public static class Options {

@Override
public PrintStream getStream() {
return Options.LogFile.getStream(defaultOptions());
return Options.LogFile.getStream();
}

/**
Expand Down Expand Up @@ -96,22 +95,57 @@ private static String makeFilename(String nameTemplate) {
* operation is performed on the stream. This is required to break a deadlock in early JVMCI
* initialization.
*/
static class DelayedOutputStream extends OutputStream {
class DelayedOutputStream extends OutputStream {
private volatile OutputStream lazy;

private OutputStream lazy() {
if (lazy == null) {
synchronized (this) {
if (lazy == null) {
String nameTemplate = LogStreamOptionKey.this.getValue(defaultOptions());
if (nameTemplate != null) {
String name = makeFilename(nameTemplate);
try {
final boolean enableAutoflush = true;
FileOutputStream result = new FileOutputStream(name);
if (!Services.IS_IN_NATIVE_IMAGE) {
printVMConfig(enableAutoflush, result);
} else {
// There are no VM arguments for the libgraal library.
}
lazy = result;
return lazy;
} catch (FileNotFoundException e) {
throw new RuntimeException("couldn't open file: " + name, e);
}
}

lazy = HotSpotJVMCIRuntime.runtime().getLogStream();
PrintStream ps = new PrintStream(lazy);
ps.printf("[Use -D%sLogFile=<path> to redirect Graal log output to a file.]%n", GRAAL_OPTION_PROPERTY_PREFIX);
ps.flush();
}
}
}
return lazy;
}

@SuppressFBWarnings(value = "DLS_DEAD_LOCAL_STORE", justification = "false positive on dead store to `ps`")
private void printVMConfig(final boolean enableAutoflush, FileOutputStream result) {
/*
* Add the JVM and Java arguments to the log file to help identity it.
*/
PrintStream ps = new PrintStream(result, enableAutoflush);
List<String> inputArguments = GraalServices.getInputArguments();
if (inputArguments != null) {
ps.println("VM Arguments: " + String.join(" ", inputArguments));
}
String cmd = Services.getSavedProperties().get("sun.java.command");
if (cmd != null) {
ps.println("sun.java.command=" + cmd);
}
}

@Override
public void write(byte[] b, int off, int len) throws IOException {
lazy().write(b, off, len);
Expand All @@ -137,32 +171,8 @@ public void close() throws IOException {
* Gets the print stream configured by this option. If no file is configured, the print
* stream will output to HotSpot's {@link HotSpotJVMCIRuntime#getLogStream() log} stream.
*/
@SuppressFBWarnings(value = "DLS_DEAD_LOCAL_STORE", justification = "false positive on dead store to `ps`")
public PrintStream getStream(OptionValues options) {
String nameTemplate = getValue(options);
if (nameTemplate != null) {
String name = makeFilename(nameTemplate);
try {
final boolean enableAutoflush = true;
PrintStream ps = new PrintStream(new FileOutputStream(name), enableAutoflush);
/*
* Add the JVM and Java arguments to the log file to help identity it.
*/
List<String> inputArguments = GraalServices.getInputArguments();
if (inputArguments != null) {
ps.println("VM Arguments: " + String.join(" ", inputArguments));
}
String cmd = Services.getSavedProperties().get("sun.java.command");
if (cmd != null) {
ps.println("sun.java.command=" + cmd);
}
return ps;
} catch (FileNotFoundException e) {
throw new RuntimeException("couldn't open file: " + name, e);
}
} else {
return new PrintStream(new DelayedOutputStream());
}
public PrintStream getStream() {
return new PrintStream(new DelayedOutputStream());
}
}

Expand Down
Loading

0 comments on commit 0fda852

Please sign in to comment.