From 8753babd1b7d358c140a5aa84de745476802ed87 Mon Sep 17 00:00:00 2001 From: Natalia Gavrilenko Date: Mon, 25 Nov 2024 13:47:14 +0100 Subject: [PATCH] Structured control flow --- .../visitors/spirv/VisitorOpsControlFlow.java | 4 +- .../spirv/builders/ControlFlowBuilder.java | 21 --------- .../spirv/VisitorOpsControlFlowTest.java | 45 +++---------------- .../spirv/mocks/MockControlFlowBuilder.java | 4 -- 4 files changed, 7 insertions(+), 67 deletions(-) diff --git a/dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/spirv/VisitorOpsControlFlow.java b/dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/spirv/VisitorOpsControlFlow.java index f1bae7323e..d5da2c6607 100644 --- a/dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/spirv/VisitorOpsControlFlow.java +++ b/dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/spirv/VisitorOpsControlFlow.java @@ -161,13 +161,13 @@ private Event visitIfBranch(Expression guard, String trueLabelId, String falseLa throw new ParsingException("Illegal backward jump to '%s' from a structured branch", labelId); } } - mergeLabelId = null; nextLabelId = trueLabelId; builder.setNextOps(Set.of("OpLabel")); Label falseLabel = cfBuilder.getOrCreateLabel(falseLabelId); - Label mergeLabel = cfBuilder.createMergeLabel(falseLabelId); + Label mergeLabel = cfBuilder.getOrCreateLabel(mergeLabelId); Event event = EventFactory.newIfJumpUnless(guard, falseLabel, mergeLabel); builder.addEvent(event); + mergeLabelId = null; return cfBuilder.endBlock(event); } diff --git a/dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/spirv/builders/ControlFlowBuilder.java b/dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/spirv/builders/ControlFlowBuilder.java index 7e10dee763..bb24422f86 100644 --- a/dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/spirv/builders/ControlFlowBuilder.java +++ b/dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/spirv/builders/ControlFlowBuilder.java @@ -15,7 +15,6 @@ public class ControlFlowBuilder { protected final Map blockLabels = new HashMap<>(); protected final Map lastBlockEvents = new HashMap<>(); - protected final Map mergeLabelIds = new HashMap<>(); protected final Deque blockStack = new ArrayDeque<>(); protected final Map> phiDefinitions = new HashMap<>(); protected final Map phiDefinitionLocations = new HashMap<>(); @@ -44,8 +43,6 @@ public void build() { if (loc != null) { event.setMetadata(loc); } lastBlockEvents.get(blockId).getPredecessor().insertAfter(event); })); - mergeLabelIds.forEach((jumpLabelId, endLabelId) -> - lastBlockEvents.get(jumpLabelId).getPredecessor().insertAfter(blockLabels.get(endLabelId))); } public void startBlock(String id) { @@ -68,12 +65,6 @@ public Label getOrCreateLabel(String id) { return blockLabels.computeIfAbsent(id, EventFactory::newLabel); } - public Label createMergeLabel(String id) { - String mergeId = id + "_end"; - mergeLabelIds.put(id, mergeId); - return createLabel(mergeId); - } - public void addPhiDefinition(String blockId, Register register, String expressionId) { phiDefinitions.computeIfAbsent(blockId, k -> new HashMap<>()).put(register, expressionId); } @@ -121,11 +112,6 @@ private void validateBeforeBuild() { throw new ParsingException("Phi operation(s) refer to undefined block(s) %s", String.join(", ", missingPhiBlocks)); } - Set missingMergeBlocks = Sets.difference(mergeLabelIds.keySet(), blockLabels.keySet()); - if (!missingMergeBlocks.isEmpty()) { - throw new ParsingException("Branch merge label(s) refer to undefined block(s) %s", - String.join(", ", missingMergeBlocks)); - } Map reverse = new HashMap<>(); lastBlockEvents.forEach((k, v) -> { if (reverse.containsKey(v)) { @@ -135,13 +121,6 @@ private void validateBeforeBuild() { }); } - private Label createLabel(String id) { - if (blockLabels.containsKey(id)) { - throw new ParsingException("Attempt to redefine label '%s'", id); - } - return getOrCreateLabel(id); - } - private SourceLocation getPhiLocation(String blockId, Register register) { String id = phiDefinitionIds.get(blockId).get(register); if (id != null) { diff --git a/dartagnan/src/test/java/com/dat3m/dartagnan/parsers/program/visitors/spirv/VisitorOpsControlFlowTest.java b/dartagnan/src/test/java/com/dat3m/dartagnan/parsers/program/visitors/spirv/VisitorOpsControlFlowTest.java index 39f9212d2e..3f619828fb 100644 --- a/dartagnan/src/test/java/com/dat3m/dartagnan/parsers/program/visitors/spirv/VisitorOpsControlFlowTest.java +++ b/dartagnan/src/test/java/com/dat3m/dartagnan/parsers/program/visitors/spirv/VisitorOpsControlFlowTest.java @@ -161,13 +161,12 @@ public void testStructuredBranch() { assertEquals("%label0", label0.getName()); assertEquals("%label2", ifJump.getLabel().getName()); - assertEquals("%label2_end", ifJump.getEndIf().getName()); + assertEquals("%label2", ifJump.getEndIf().getName()); assertEquals("%label1", label1.getName()); assertEquals("%label2", jump.getLabel().getName()); assertEquals("%label2", label2.getName()); assertTrue(jump.isGoto()); - assertEquals(Map.of("%label2", "%label2_end"), cfBuilder.getMergeLabelIds()); assertEquals(Map.of("%label0", ifJump, "%label1", jump, "%label2", ret), cfBuilder.getLastBlockEvents()); } @@ -213,10 +212,10 @@ public void testStructuredBranchNestedTrue() { assertEquals("%label0", label0.getName()); assertEquals("%label2", ifJump.getLabel().getName()); - assertEquals("%label2_end", ifJump.getEndIf().getName()); + assertEquals("%label2", ifJump.getEndIf().getName()); assertEquals("%label1", label1.getName()); assertEquals("%label2_inner", ifJumpInner.getLabel().getName()); - assertEquals("%label2_inner_end", ifJumpInner.getEndIf().getName()); + assertEquals("%label2_inner", ifJumpInner.getEndIf().getName()); assertEquals("%label1_inner", label1Inner.getName()); assertEquals("%label2_inner", jumpInner.getLabel().getName()); assertEquals("%label2_inner", label2Inner.getName()); @@ -281,12 +280,12 @@ public void testStructuredBranchNestedFalse() { assertEquals("%label0", label0.getName()); assertEquals("%label3", ifJump.getLabel().getName()); - assertEquals("%label3_end", ifJump.getEndIf().getName()); + assertEquals("%label2", ifJump.getEndIf().getName()); assertEquals("%label2", jump1.getLabel().getName()); assertEquals("%label1", label1.getName()); assertEquals("%label2", label2.getName()); assertEquals("%label2_inner", ifJumpInner.getLabel().getName()); - assertEquals("%label2_inner_end", ifJumpInner.getEndIf().getName()); + assertEquals("%label2_inner", ifJumpInner.getEndIf().getName()); assertEquals("%label1_inner", label1Inner.getName()); assertEquals("%label2_inner", jumpInner.getLabel().getName()); assertEquals("%label2_inner", label2Inner.getName()); @@ -307,36 +306,6 @@ public void testStructuredBranchNestedFalse() { ), cfBuilder.getLastBlockEvents()); } - @Test - public void testStructuredBranchNestedSameLabel() { - // given - String input = """ - %label0 = OpLabel - OpSelectionMerge %label2 None - OpBranchConditional %value %label1 %label2 - %label1 = OpLabel - OpSelectionMerge %label2 None - OpBranchConditional %value %label1_inner %label2 - %label1_inner = OpLabel - OpBranch %label2 - %label2 = OpLabel - OpReturn - """; - - builder.mockFunctionStart(false); - builder.mockBoolType("%bool"); - builder.mockUndefinedValue("%value", "%bool"); - - try { - // when - visit(input); - fail("Should throw exception"); - } catch (ParsingException e) { - // then - assertEquals("Attempt to redefine label '%label2_end'", e.getMessage()); - } - } - @Test public void testLoopWithForwardLabels() { // given @@ -379,8 +348,6 @@ public void testLoopWithForwardLabels() { assertTrue(jump2.isGoto()); assertTrue(jump3.isGoto()); - assertTrue(cfBuilder.getMergeLabelIds().isEmpty()); - assertEquals(Map.of("%label0", jump1, "%label1", jump3, "%label2", ret), cfBuilder.getLastBlockEvents()); } @@ -420,8 +387,6 @@ public void testLoopWithBackwardLabel() { assertFalse(jump1.isGoto()); assertTrue(jump2.isGoto()); - assertTrue(cfBuilder.getMergeLabelIds().isEmpty()); - assertEquals(Map.of("%label0", jump1, "%label1", ret), cfBuilder.getLastBlockEvents()); } diff --git a/dartagnan/src/test/java/com/dat3m/dartagnan/parsers/program/visitors/spirv/mocks/MockControlFlowBuilder.java b/dartagnan/src/test/java/com/dat3m/dartagnan/parsers/program/visitors/spirv/mocks/MockControlFlowBuilder.java index 02506234f3..f404985793 100644 --- a/dartagnan/src/test/java/com/dat3m/dartagnan/parsers/program/visitors/spirv/mocks/MockControlFlowBuilder.java +++ b/dartagnan/src/test/java/com/dat3m/dartagnan/parsers/program/visitors/spirv/mocks/MockControlFlowBuilder.java @@ -18,10 +18,6 @@ public List getBlockStack() { return blockStack.stream().toList(); } - public Map getMergeLabelIds() { - return Map.copyOf(mergeLabelIds); - } - public Map getLastBlockEvents() { return Map.copyOf(lastBlockEvents); }