Delay insertion of membars for volatile accesses

zcfrank1st · Oct 16, 2019 · e069afe · e069afe
1 parent 49904bd
commit e069afe
Show file tree

Hide file tree

Showing 70 changed files with 580 additions and 101 deletions.
diff --git a/....graalvm.compiler.core.amd64/src/org/graalvm/compiler/core/amd64/AMD64NodeMatchRules.java b/....graalvm.compiler.core.amd64/src/org/graalvm/compiler/core/amd64/AMD64NodeMatchRules.java
@@ -359,6 +359,7 @@ public ComplexMatchResult resetLowestSetBit(ValueNode a, ValueNode b) {
 
     @MatchRule("(If (IntegerTest Read=access value))")
     @MatchRule("(If (IntegerTest FloatingRead=access value))")
+    @MatchRule("(If (IntegerTest VolatileRead=access value))")
     public ComplexMatchResult integerTestBranchMemory(IfNode root, LIRLowerableAccess access, ValueNode value) {
         return emitIntegerTestBranchMemory(root, value, access);
     }
@@ -369,14 +370,21 @@ public ComplexMatchResult integerTestBranchMemory(IfNode root, LIRLowerableAcces
     @MatchRule("(If (IntegerEquals=compare value FloatingRead=access))")
     @MatchRule("(If (IntegerLessThan=compare value FloatingRead=access))")
     @MatchRule("(If (IntegerBelow=compare value FloatingRead=access))")
+    @MatchRule("(If (IntegerEquals=compare value VolatileRead=access))")
+    @MatchRule("(If (IntegerLessThan=compare value VolatileRead=access))")
+    @MatchRule("(If (IntegerBelow=compare value VolatileRead=access))")
     @MatchRule("(If (FloatEquals=compare value Read=access))")
     @MatchRule("(If (FloatEquals=compare value FloatingRead=access))")
+    @MatchRule("(If (FloatEquals=compare value VolatileRead=access))")
     @MatchRule("(If (FloatLessThan=compare value Read=access))")
     @MatchRule("(If (FloatLessThan=compare value FloatingRead=access))")
+    @MatchRule("(If (FloatLessThan=compare value VolatileRead=access))")
     @MatchRule("(If (PointerEquals=compare value Read=access))")
     @MatchRule("(If (PointerEquals=compare value FloatingRead=access))")
+    @MatchRule("(If (PointerEquals=compare value VolatileRead=access))")
     @MatchRule("(If (ObjectEquals=compare value Read=access))")
     @MatchRule("(If (ObjectEquals=compare value FloatingRead=access))")
+    @MatchRule("(If (ObjectEquals=compare value VolatileRead=access))")
     public ComplexMatchResult ifCompareMemory(IfNode root, CompareNode compare, ValueNode value, LIRLowerableAccess access) {
         return emitCompareBranchMemory(root, compare, value, access);
     }
@@ -478,6 +486,7 @@ private ComplexMatchResult binaryRead(AMD64Assembler.VexRVMOp op, OperandSize si
 
     @MatchRule("(Add value Read=access)")
     @MatchRule("(Add value FloatingRead=access)")
+    @MatchRule("(Add value VolatileRead=access)")
     public ComplexMatchResult addMemory(ValueNode value, LIRLowerableAccess access) {
         OperandSize size = getMemorySize(access);
         if (size.isXmmType()) {
@@ -493,6 +502,7 @@ public ComplexMatchResult addMemory(ValueNode value, LIRLowerableAccess access)
 
     @MatchRule("(Sub value Read=access)")
     @MatchRule("(Sub value FloatingRead=access)")
+    @MatchRule("(Sub value VolatileRead=access)")
     public ComplexMatchResult subMemory(ValueNode value, LIRLowerableAccess access) {
         OperandSize size = getMemorySize(access);
         if (size.isXmmType()) {
@@ -508,6 +518,7 @@ public ComplexMatchResult subMemory(ValueNode value, LIRLowerableAccess access)
 
     @MatchRule("(Mul value Read=access)")
     @MatchRule("(Mul value FloatingRead=access)")
+    @MatchRule("(Mul value VolatileRead=access)")
     public ComplexMatchResult mulMemory(ValueNode value, LIRLowerableAccess access) {
         OperandSize size = getMemorySize(access);
         if (size.isXmmType()) {
@@ -523,6 +534,7 @@ public ComplexMatchResult mulMemory(ValueNode value, LIRLowerableAccess access)
 
     @MatchRule("(And value Read=access)")
     @MatchRule("(And value FloatingRead=access)")
+    @MatchRule("(And value VolatileRead=access)")
     public ComplexMatchResult andMemory(ValueNode value, LIRLowerableAccess access) {
         OperandSize size = getMemorySize(access);
         if (size.isXmmType()) {
@@ -534,6 +546,7 @@ public ComplexMatchResult andMemory(ValueNode value, LIRLowerableAccess access)
 
     @MatchRule("(Or value Read=access)")
     @MatchRule("(Or value FloatingRead=access)")
+    @MatchRule("(Or value VolatileRead=access)")
     public ComplexMatchResult orMemory(ValueNode value, LIRLowerableAccess access) {
         OperandSize size = getMemorySize(access);
         if (size.isXmmType()) {
@@ -545,6 +558,7 @@ public ComplexMatchResult orMemory(ValueNode value, LIRLowerableAccess access) {
 
     @MatchRule("(Xor value Read=access)")
     @MatchRule("(Xor value FloatingRead=access)")
+    @MatchRule("(Xor value VolatileRead=access)")
     public ComplexMatchResult xorMemory(ValueNode value, LIRLowerableAccess access) {
         OperandSize size = getMemorySize(access);
         if (size.isXmmType()) {
@@ -565,19 +579,22 @@ public ComplexMatchResult writeNarrow(WriteNode root, NarrowNode narrow) {
 
     @MatchRule("(SignExtend Read=access)")
     @MatchRule("(SignExtend FloatingRead=access)")
+    @MatchRule("(SignExtend VolatileRead=access)")
     public ComplexMatchResult signExtend(SignExtendNode root, LIRLowerableAccess access) {
         return emitSignExtendMemory(access, root.getInputBits(), root.getResultBits(), null);
     }
 
     @MatchRule("(ZeroExtend Read=access)")
     @MatchRule("(ZeroExtend FloatingRead=access)")
+    @MatchRule("(ZeroExtend VolatileRead=access)")
     public ComplexMatchResult zeroExtend(ZeroExtendNode root, LIRLowerableAccess access) {
         AMD64Kind memoryKind = getMemoryKind(access);
         return builder -> getArithmeticLIRGenerator().emitZeroExtendMemory(memoryKind, root.getResultBits(), (AMD64AddressValue) operand(access.getAddress()), getState(access));
     }
 
     @MatchRule("(Narrow Read=access)")
     @MatchRule("(Narrow FloatingRead=access)")
+    @MatchRule("(Narrow VolatileRead=access)")
     public ComplexMatchResult narrowRead(NarrowNode root, LIRLowerableAccess access) {
         return new ComplexMatchResult() {
             @Override
@@ -595,13 +612,15 @@ public Value evaluate(NodeLIRBuilder builder) {
 
     @MatchRule("(SignExtend (Narrow=narrow Read=access))")
     @MatchRule("(SignExtend (Narrow=narrow FloatingRead=access))")
+    @MatchRule("(SignExtend (Narrow=narrow VolatileRead=access))")
     public ComplexMatchResult signExtendNarrowRead(SignExtendNode root, NarrowNode narrow, LIRLowerableAccess access) {
         LIRKind kind = getLIRGeneratorTool().getLIRKind(narrow.stamp(NodeView.DEFAULT));
         return emitSignExtendMemory(access, narrow.getResultBits(), root.getResultBits(), kind);
     }
 
     @MatchRule("(FloatConvert Read=access)")
     @MatchRule("(FloatConvert FloatingRead=access)")
+    @MatchRule("(FloatConvert VolatileRead=access)")
     public ComplexMatchResult floatConvert(FloatConvertNode root, LIRLowerableAccess access) {
         switch (root.getFloatConvert()) {
             case D2F:
@@ -631,6 +650,7 @@ public ComplexMatchResult floatConvert(FloatConvertNode root, LIRLowerableAccess
 
     @MatchRule("(Reinterpret Read=access)")
     @MatchRule("(Reinterpret FloatingRead=access)")
+    @MatchRule("(Reinterpret VolatileRead=access)")
     public ComplexMatchResult reinterpret(ReinterpretNode root, LIRLowerableAccess access) {
         return builder -> {
             LIRKind kind = getLIRGeneratorTool().getLIRKind(root.stamp(NodeView.DEFAULT));

diff --git a/...c/org.graalvm.compiler.core.common/src/org/graalvm/compiler/core/common/GraalOptions.java b/...c/org.graalvm.compiler.core.common/src/org/graalvm/compiler/core/common/GraalOptions.java
@@ -291,4 +291,7 @@ public final class GraalOptions {
 
     @Option(help = "Alignment in bytes for loop header blocks.", type = OptionType.Expert)
     public static final OptionKey<Integer> LoopHeaderAlignment = new OptionKey<>(16);
+
+    @Option(help = "Do not include membars for volatile accesses until the end of optimizations.", type = OptionType.Expert)
+    public static final OptionKey<Boolean> LateMembars = new OptionKey<>(true);
 }
diff --git a/....graalvm.compiler.core.sparc/src/org/graalvm/compiler/core/sparc/SPARCNodeMatchRules.java b/....graalvm.compiler.core.sparc/src/org/graalvm/compiler/core/sparc/SPARCNodeMatchRules.java
@@ -134,12 +134,14 @@ private ComplexMatchResult emitZeroExtendMemory(Access access, int fromBits, int
 
     @MatchRule("(SignExtend Read=access)")
     @MatchRule("(SignExtend FloatingRead=access)")
+    @MatchRule("(SignExtend VolatileRead=access)")
     public ComplexMatchResult signExtend(SignExtendNode root, Access access) {
         return emitSignExtendMemory(access, root.getInputBits(), root.getResultBits());
     }
 
     @MatchRule("(ZeroExtend Read=access)")
     @MatchRule("(ZeroExtend FloatingRead=access)")
+    @MatchRule("(ZeroExtend VolatileRead=access)")
     public ComplexMatchResult zeroExtend(ZeroExtendNode root, Access access) {
         return emitZeroExtendMemory(access, root.getInputBits(), root.getResultBits());
     }

diff --git a/.../org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/GraalCompilerTest.java b/.../org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/GraalCompilerTest.java
@@ -1119,6 +1119,12 @@ protected StructuredGraph getFinalGraph(ResolvedJavaMethod method) {
         return graph;
     }
 
+    protected StructuredGraph getFinalGraph(ResolvedJavaMethod method, OptionValues options) {
+        StructuredGraph graph = parseForCompile(method, options);
+        applyFrontEnd(graph);
+        return graph;
+    }
+
     @SuppressWarnings("try")
     protected void applyFrontEnd(StructuredGraph graph) {
         DebugContext debug = graph.getDebug();

diff --git a/compiler/src/org.graalvm.compiler.core/src/org/graalvm/compiler/core/gen/NodeMatchRules.java b/compiler/src/org.graalvm.compiler.core/src/org/graalvm/compiler/core/gen/NodeMatchRules.java
@@ -63,6 +63,7 @@
 import org.graalvm.compiler.nodes.java.ValueCompareAndSwapNode;
 import org.graalvm.compiler.nodes.memory.FloatingReadNode;
 import org.graalvm.compiler.nodes.memory.ReadNode;
+import org.graalvm.compiler.nodes.memory.VolatileReadNode;
 import org.graalvm.compiler.nodes.memory.WriteNode;
 
 import jdk.vm.ci.meta.Value;
@@ -75,6 +76,7 @@
 @MatchableNode(nodeClass = LeftShiftNode.class, inputs = {"x", "y"}, ignoresSideEffects = true)
 @MatchableNode(nodeClass = NarrowNode.class, inputs = {"value"}, ignoresSideEffects = true)
 @MatchableNode(nodeClass = ReadNode.class, inputs = {"address"})
+@MatchableNode(nodeClass = VolatileReadNode.class, inputs = {"address"})
 @MatchableNode(nodeClass = ReinterpretNode.class, inputs = {"value"}, ignoresSideEffects = true)
 @MatchableNode(nodeClass = SignExtendNode.class, inputs = {"value"}, ignoresSideEffects = true)
 @MatchableNode(nodeClass = UnsignedRightShiftNode.class, inputs = {"x", "y"}, ignoresSideEffects = true)

diff --git a/compiler/src/org.graalvm.compiler.core/src/org/graalvm/compiler/core/phases/LowTier.java b/compiler/src/org.graalvm.compiler.core/src/org/graalvm/compiler/core/phases/LowTier.java
@@ -24,6 +24,7 @@
  */
 package org.graalvm.compiler.core.phases;
 
+import static org.graalvm.compiler.core.common.GraalOptions.LateMembars;
 import static org.graalvm.compiler.phases.common.DeadCodeEliminationPhase.Optionality.Required;
 
 import org.graalvm.compiler.core.common.GraalOptions;
@@ -36,6 +37,7 @@
 import org.graalvm.compiler.phases.common.DeadCodeEliminationPhase;
 import org.graalvm.compiler.phases.common.ExpandLogicPhase;
 import org.graalvm.compiler.phases.common.FixReadsPhase;
+import org.graalvm.compiler.phases.common.InsertMembarsPhase;
 import org.graalvm.compiler.phases.common.LoweringPhase;
 import org.graalvm.compiler.phases.common.ProfileCompiledMethodsPhase;
 import org.graalvm.compiler.phases.common.PropagateDeoptimizeProbabilityPhase;
@@ -78,6 +80,9 @@ public LowTier(OptionValues options) {
 
         appendPhase(new PropagateDeoptimizeProbabilityPhase());
 
+        if (LateMembars.getValue(options)) {
+            appendPhase(new InsertMembarsPhase());
+        }
         appendPhase(new SchedulePhase(SchedulePhase.SchedulingStrategy.LATEST_OUT_OF_LOOPS));
     }
 }