From 06fdc64497ec14fcce66d31c48c1d637b427d224 Mon Sep 17 00:00:00 2001 From: flofriday Date: Tue, 12 May 2026 18:38:50 +0200 Subject: [PATCH 01/11] frontend: Refactor AST children resolution for performance This approach allocates fewer intermediate lists. --- .../ast/ChildAnnotationProcessor.java | 82 ++-- vadl/main/vadl/ast/Ast.java | 17 +- vadl/main/vadl/ast/AstVisitor.java | 436 +++++------------- vadl/main/vadl/ast/Definition.java | 36 +- vadl/main/vadl/ast/Expr.java | 35 +- vadl/main/vadl/ast/Statement.java | 21 +- vadl/main/vadl/ast/SymbolTable.java | 38 +- 7 files changed, 217 insertions(+), 448 deletions(-) diff --git a/java-annotations/main/vadl/javaannotations/ast/ChildAnnotationProcessor.java b/java-annotations/main/vadl/javaannotations/ast/ChildAnnotationProcessor.java index b4cf0597f..f0f4e84d7 100644 --- a/java-annotations/main/vadl/javaannotations/ast/ChildAnnotationProcessor.java +++ b/java-annotations/main/vadl/javaannotations/ast/ChildAnnotationProcessor.java @@ -176,18 +176,16 @@ private void generateNodeChildrenRegistry() throws IOException { out.println("// Generated code from %s".formatted(this.getClass().getName())); out.println("package " + packageName + ";"); out.println(); - out.println("import java.util.List;"); - out.println("import java.util.ArrayList;"); - out.println("import java.util.Collections;"); out.println("import java.util.Map;"); out.println("import java.util.HashMap;"); - out.println("import java.util.function.Function;"); + out.println("import java.util.function.BiConsumer;"); + out.println("import java.util.function.Consumer;"); out.println(); // Generate registry class out.println("public final class NodeChildrenRegistry {"); out.println( - " private static final Map, Function>> COLLECTORS = " + " private static final Map, BiConsumer>> COLLECTORS = " + "new HashMap<>();"); out.println(); @@ -202,70 +200,41 @@ private void generateNodeChildrenRegistry() throws IOException { String className = classElement.getQualifiedName().toString(); List childFields = entry.getValue(); - // Optimization for when all children are already in a single list - if (childFields.size() == 1 && isFieldList(childFields.getFirst())) { - var field = childFields.getFirst(); - out.println( - " COLLECTORS.put(" + className + ".class, (node) -> (List) (List) ((" - + className + ") node)." + fieldAccessor(field) + ");"); - continue; - } - - out.println(" COLLECTORS.put(" + className + ".class, (node) -> {"); + out.println(" COLLECTORS.put(" + className + ".class, (node, action) -> {"); out.println(" " + className + " n = (" + className + ") node;"); - // Calculate the capacity to optimize allocations - out.println(" int capacity = 0"); - for (VariableElement field : childFields) { - String fieldName = field.getSimpleName().toString(); - if (isFieldList(field)) { - out.println( - " + n.%s.size() // %s".formatted(fieldAccessor(field), fieldName)); - } else { - out.println(" + 1 // %s".formatted(fieldName)); - } - } - out.println(" ;"); - - - out.println(" List children = new ArrayList<>(capacity);"); - - // Add code to collect children for this node type for (VariableElement field : childFields) { - // Handle different field types if (isFieldList(field)) { - out.println(" if ( n." + fieldAccessor(field) + " != null) {"); - out.println(" for (var child: n." + fieldAccessor(field) + ") {"); - out.println(" children.add((Node) child);"); - out.println(" }"); + out.println(" if (n." + fieldAccessor(field) + " != null) {"); + out.println(" for (var child : n." + fieldAccessor(field) + ") {"); + out.println(" action.accept((Node) child);"); + out.println(" }"); out.println(" }"); } else { - out.println(" if ( n." + fieldAccessor(field) + " != null) {"); - out.println(" children.add((Node) n." + fieldAccessor(field) + ");"); + out.println(" if (n." + fieldAccessor(field) + " != null) {"); + out.println(" action.accept((Node) n." + fieldAccessor(field) + ");"); out.println(" }"); } } - out.println(" return children;"); out.println(" });"); } out.println(" }"); out.println(); - // Method to get children for any node - out.println(" public static List getChildren(Node node) {"); + // Method to iterate children for any node + out.println( + " public static void forEachChild(Node node, Consumer action) {"); out.println( - " Function> collector = COLLECTORS.get(node.getClass());"); + " BiConsumer> collector = COLLECTORS.get(node.getClass());"); out.println(" if (collector != null) {"); - out.println(" return collector.apply(node);"); + out.println(" collector.accept(node, action);"); out.println(" }"); - out.println(" return Collections.emptyList();"); out.println(" }"); out.println(""); - - // Method to get all children but specify the exact class, only used for edgecases + // Method to iterate children but specify the exact class, only used for edge cases out.println(" /**"); out.println(" * Specify the class directly as which it should be loaded."); out.println( @@ -273,20 +242,21 @@ private void generateNodeChildrenRegistry() throws IOException { + "to get the children"); out.println(" * from your superclass."); out.println(" *"); - out.println(" * @param node from which the children are loaded."); + out.println(" * @param node from which the children are iterated."); out.println(" * @param nodeType as which the node should be interpreted."); - out.println(" * @return the children."); + out.println(" * @param action called for each child."); out.println(" */"); out.println( - " public static List unsafeGetChildrenDirect(Node node, " - + "Class nodeType) {"); - out.println(" Function> collector = COLLECTORS.get(nodeType);"); - out.println(" if (collector == null) {"); + " public static void unsafeForEachChildDirect(Node node, " + + "Class nodeType, Consumer action) {"); + out.println( + " BiConsumer> collector = COLLECTORS.get(nodeType);"); + out.println(" if (collector == null) {"); out.println( - " throw new IllegalArgumentException(\"Node type \" + nodeType + \" " + " throw new IllegalArgumentException(\"Node type \" + nodeType + \" " + "not supported\");"); - out.println(" }"); - out.println(" return collector.apply(node);"); + out.println(" }"); + out.println(" collector.accept(node, action);"); out.println(" }"); out.println("}"); } diff --git a/vadl/main/vadl/ast/Ast.java b/vadl/main/vadl/ast/Ast.java index 64c70b56a..dbdcca7b4 100644 --- a/vadl/main/vadl/ast/Ast.java +++ b/vadl/main/vadl/ast/Ast.java @@ -22,6 +22,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.function.Consumer; import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -210,6 +211,16 @@ static boolean isBlockLayout(Node n) { abstract void prettyPrint(int indent, StringBuilder builder); + /** + * Invokes {@code action} for each child owned by this node. + * + *

Either automatically generated by annotating children with the + * {@link vadl.javaannotations.ast.Child} annotation or by overriding this method. + */ + void forEachChild(Consumer action) { + NodeChildrenRegistry.forEachChild(this, action); + } + /** * Returns all children "owned" by this node. * @@ -218,8 +229,10 @@ static boolean isBlockLayout(Node n) { * * @return a list of children. */ - List children() { - return NodeChildrenRegistry.getChildren(this); + final List children() { + var result = new ArrayList(); + forEachChild(result::add); + return result; } @Override diff --git a/vadl/main/vadl/ast/AstVisitor.java b/vadl/main/vadl/ast/AstVisitor.java index b4b5c4309..6153f5fd8 100644 --- a/vadl/main/vadl/ast/AstVisitor.java +++ b/vadl/main/vadl/ast/AstVisitor.java @@ -63,17 +63,13 @@ protected final void travel(Node node) { } // If it's just a intermediate node, just visit it's children - for (var child : node.children()) { - travel(child); - } + node.forEachChild(this::travel); } @Override public Void visit(AbiSequenceDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -81,9 +77,7 @@ public Void visit(AbiSequenceDefinition definition) { @Override public Void visit(AliasDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -91,9 +85,7 @@ public Void visit(AliasDefinition definition) { @Override public Void visit(AnnotationDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -101,9 +93,7 @@ public Void visit(AnnotationDefinition definition) { @Override public Void visit(ApplicationBinaryInterfaceDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -111,9 +101,7 @@ public Void visit(ApplicationBinaryInterfaceDefinition definition) { @Override public Void visit(AsmDescriptionDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -121,9 +109,7 @@ public Void visit(AsmDescriptionDefinition definition) { @Override public Void visit(AsmDirectiveDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -131,9 +117,7 @@ public Void visit(AsmDirectiveDefinition definition) { @Override public Void visit(AsmGrammarAlternativesDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -141,9 +125,7 @@ public Void visit(AsmGrammarAlternativesDefinition definition) { @Override public Void visit(AsmGrammarElementDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -151,9 +133,7 @@ public Void visit(AsmGrammarElementDefinition definition) { @Override public Void visit(AsmGrammarLiteralDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -161,9 +141,7 @@ public Void visit(AsmGrammarLiteralDefinition definition) { @Override public Void visit(AsmGrammarLocalVarDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -171,9 +149,7 @@ public Void visit(AsmGrammarLocalVarDefinition definition) { @Override public Void visit(AsmGrammarRuleDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -181,9 +157,7 @@ public Void visit(AsmGrammarRuleDefinition definition) { @Override public Void visit(AsmGrammarTypeDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -191,9 +165,7 @@ public Void visit(AsmGrammarTypeDefinition definition) { @Override public Void visit(AsmModifierDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -201,9 +173,7 @@ public Void visit(AsmModifierDefinition definition) { @Override public Void visit(AssemblyDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -211,9 +181,7 @@ public Void visit(AssemblyDefinition definition) { @Override public Void visit(CacheDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -221,9 +189,7 @@ public Void visit(CacheDefinition definition) { @Override public Void visit(ConstantDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -231,9 +197,7 @@ public Void visit(ConstantDefinition definition) { @Override public Void visit(CounterDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -241,9 +205,7 @@ public Void visit(CounterDefinition definition) { @Override public Void visit(CpuFunctionDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -251,9 +213,7 @@ public Void visit(CpuFunctionDefinition definition) { @Override public Void visit(CpuMemoryRegionDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -261,9 +221,7 @@ public Void visit(CpuMemoryRegionDefinition definition) { @Override public Void visit(CpuProcessDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -271,9 +229,7 @@ public Void visit(CpuProcessDefinition definition) { @Override public Void visit(DefinitionList definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -281,9 +237,7 @@ public Void visit(DefinitionList definition) { @Override public Void visit(EncodingDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -291,9 +245,7 @@ public Void visit(EncodingDefinition definition) { @Override public Void visit(EnumerationDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -301,9 +253,7 @@ public Void visit(EnumerationDefinition definition) { @Override public Void visit(ExceptionDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -311,9 +261,7 @@ public Void visit(ExceptionDefinition definition) { @Override public Void visit(FormatDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -321,9 +269,7 @@ public Void visit(FormatDefinition definition) { @Override public Void visit(DerivedFormatField definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -331,9 +277,7 @@ public Void visit(DerivedFormatField definition) { @Override public Void visit(RangeFormatField definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -341,9 +285,7 @@ public Void visit(RangeFormatField definition) { @Override public Void visit(TypedFormatField definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -351,9 +293,7 @@ public Void visit(TypedFormatField definition) { @Override public Void visit(EncodingFormatField definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -361,9 +301,7 @@ public Void visit(EncodingFormatField definition) { @Override public Void visit(PredicateFormatField definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -371,9 +309,7 @@ public Void visit(PredicateFormatField definition) { @Override public Void visit(FunctionDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -381,9 +317,7 @@ public Void visit(FunctionDefinition definition) { @Override public Void visit(GroupDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -391,9 +325,7 @@ public Void visit(GroupDefinition definition) { @Override public Void visit(ImportDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -401,9 +333,7 @@ public Void visit(ImportDefinition definition) { @Override public Void visit(InstructionDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -411,9 +341,7 @@ public Void visit(InstructionDefinition definition) { @Override public Void visit(InstructionSetDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -421,9 +349,7 @@ public Void visit(InstructionSetDefinition definition) { @Override public Void visit(LogicDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -431,9 +357,7 @@ public Void visit(LogicDefinition definition) { @Override public Void visit(MacroInstanceDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -441,9 +365,7 @@ public Void visit(MacroInstanceDefinition definition) { @Override public Void visit(MacroInstructionDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -451,9 +373,7 @@ public Void visit(MacroInstructionDefinition definition) { @Override public Void visit(MacroMatchDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -461,9 +381,7 @@ public Void visit(MacroMatchDefinition definition) { @Override public Void visit(MemoryDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -471,9 +389,7 @@ public Void visit(MemoryDefinition definition) { @Override public Void visit(MicroArchitectureDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -481,9 +397,7 @@ public Void visit(MicroArchitectureDefinition definition) { @Override public Void visit(ProcessorDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -491,9 +405,7 @@ public Void visit(ProcessorDefinition definition) { @Override public Void visit(ModelDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -501,9 +413,7 @@ public Void visit(ModelDefinition definition) { @Override public Void visit(ModelTypeDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -511,9 +421,7 @@ public Void visit(ModelTypeDefinition definition) { @Override public Void visit(OperationDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -521,9 +429,7 @@ public Void visit(OperationDefinition definition) { @Override public Void visit(Parameter definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -531,9 +437,7 @@ public Void visit(Parameter definition) { @Override public Void visit(PatchDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -541,9 +445,7 @@ public Void visit(PatchDefinition definition) { @Override public Void visit(PipelineDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -551,9 +453,7 @@ public Void visit(PipelineDefinition definition) { @Override public Void visit(PlaceholderDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -561,9 +461,7 @@ public Void visit(PlaceholderDefinition definition) { @Override public Void visit(PortBehaviorDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -571,9 +469,7 @@ public Void visit(PortBehaviorDefinition definition) { @Override public Void visit(ProcessDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -581,9 +477,7 @@ public Void visit(ProcessDefinition definition) { @Override public Void visit(PseudoInstructionDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -591,9 +485,7 @@ public Void visit(PseudoInstructionDefinition definition) { @Override public Void visit(RecordTypeDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -601,9 +493,7 @@ public Void visit(RecordTypeDefinition definition) { @Override public Void visit(RegisterDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -611,9 +501,7 @@ public Void visit(RegisterDefinition definition) { @Override public Void visit(RelocationDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -621,9 +509,7 @@ public Void visit(RelocationDefinition definition) { @Override public Void visit(SignalDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -631,9 +517,7 @@ public Void visit(SignalDefinition definition) { @Override public Void visit(SourceDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -641,9 +525,7 @@ public Void visit(SourceDefinition definition) { @Override public Void visit(SpecialPurposeRegisterDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -651,9 +533,7 @@ public Void visit(SpecialPurposeRegisterDefinition definition) { @Override public Void visit(StageDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -661,9 +541,7 @@ public Void visit(StageDefinition definition) { @Override public Void visit(UsingDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -671,9 +549,7 @@ public Void visit(UsingDefinition definition) { @Override public Void visit(AbiClangTypeDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -681,9 +557,7 @@ public Void visit(AbiClangTypeDefinition definition) { @Override public Void visit(AbiClangNumericTypeDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -691,9 +565,7 @@ public Void visit(AbiClangNumericTypeDefinition definition) { @Override public Void visit(StageOutputDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -701,9 +573,7 @@ public Void visit(StageOutputDefinition definition) { @Override public Void visit(AbiSpecialPurposeInstructionDefinition definition) { beforeTravel(definition); - for (var child : definition.children()) { - travel(child); - } + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -711,9 +581,7 @@ public Void visit(AbiSpecialPurposeInstructionDefinition definition) { @Override public Void visit(Identifier expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -721,9 +589,7 @@ public Void visit(Identifier expr) { @Override public Void visit(BinaryExpr expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -731,9 +597,7 @@ public Void visit(BinaryExpr expr) { @Override public Void visit(GroupedExpr expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -741,9 +605,7 @@ public Void visit(GroupedExpr expr) { @Override public Void visit(IntegerLiteral expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -751,9 +613,7 @@ public Void visit(IntegerLiteral expr) { @Override public Void visit(WildcardLiteral expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -761,9 +621,7 @@ public Void visit(WildcardLiteral expr) { @Override public Void visit(BinaryLiteral expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -771,9 +629,7 @@ public Void visit(BinaryLiteral expr) { @Override public Void visit(BoolLiteral expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -781,9 +637,7 @@ public Void visit(BoolLiteral expr) { @Override public Void visit(StringLiteral expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -791,9 +645,7 @@ public Void visit(StringLiteral expr) { @Override public Void visit(PlaceholderExpr expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -801,9 +653,7 @@ public Void visit(PlaceholderExpr expr) { @Override public Void visit(MacroInstanceExpr expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -811,9 +661,7 @@ public Void visit(MacroInstanceExpr expr) { @Override public Void visit(RangeExpr expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -821,9 +669,7 @@ public Void visit(RangeExpr expr) { @Override public Void visit(TypeLiteral expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -831,9 +677,7 @@ public Void visit(TypeLiteral expr) { @Override public Void visit(IdentifierPath expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -841,9 +685,7 @@ public Void visit(IdentifierPath expr) { @Override public Void visit(UnaryExpr expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -851,9 +693,7 @@ public Void visit(UnaryExpr expr) { @Override public Void visit(CallIndexExpr expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -861,9 +701,7 @@ public Void visit(CallIndexExpr expr) { @Override public Void visit(IfExpr expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -871,9 +709,7 @@ public Void visit(IfExpr expr) { @Override public Void visit(LetExpr expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -881,9 +717,7 @@ public Void visit(LetExpr expr) { @Override public Void visit(CastExpr expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -891,9 +725,7 @@ public Void visit(CastExpr expr) { @Override public Void visit(SymbolExpr expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -901,9 +733,7 @@ public Void visit(SymbolExpr expr) { @Override public Void visit(MacroMatchExpr expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -911,9 +741,7 @@ public Void visit(MacroMatchExpr expr) { @Override public Void visit(MatchExpr expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -921,9 +749,7 @@ public Void visit(MatchExpr expr) { @Override public Void visit(AsIdExpr expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -931,9 +757,7 @@ public Void visit(AsIdExpr expr) { @Override public Void visit(AsStrExpr expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -941,9 +765,7 @@ public Void visit(AsStrExpr expr) { @Override public Void visit(ExistsInExpr expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -951,9 +773,7 @@ public Void visit(ExistsInExpr expr) { @Override public Void visit(ExistsInThenExpr expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -961,9 +781,7 @@ public Void visit(ExistsInThenExpr expr) { @Override public Void visit(ForallExpr expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -971,9 +789,7 @@ public Void visit(ForallExpr expr) { @Override public Void visit(SequenceCallExpr expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -981,9 +797,7 @@ public Void visit(SequenceCallExpr expr) { @Override public Void visit(ExpandedSequenceCallExpr expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -991,9 +805,7 @@ public Void visit(ExpandedSequenceCallExpr expr) { @Override public Void visit(ExpandedAliasDefSequenceCallExpr expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -1001,9 +813,7 @@ public Void visit(ExpandedAliasDefSequenceCallExpr expr) { @Override public Void visit(ResourceReferenceExression expr) { beforeTravel(expr); - for (var child : expr.children()) { - travel(child); - } + expr.forEachChild(this::travel); afterTravel(expr); return null; } @@ -1011,9 +821,7 @@ public Void visit(ResourceReferenceExression expr) { @Override public Void visit(AssignmentStatement statement) { beforeTravel(statement); - for (var child : statement.children()) { - travel(child); - } + statement.forEachChild(this::travel); afterTravel(statement); return null; } @@ -1021,9 +829,7 @@ public Void visit(AssignmentStatement statement) { @Override public Void visit(BlockStatement statement) { beforeTravel(statement); - for (var child : statement.children()) { - travel(child); - } + statement.forEachChild(this::travel); afterTravel(statement); return null; } @@ -1031,9 +837,7 @@ public Void visit(BlockStatement statement) { @Override public Void visit(CallStatement statement) { beforeTravel(statement); - for (var child : statement.children()) { - travel(child); - } + statement.forEachChild(this::travel); afterTravel(statement); return null; } @@ -1041,9 +845,7 @@ public Void visit(CallStatement statement) { @Override public Void visit(ForallStatement statement) { beforeTravel(statement); - for (var child : statement.children()) { - travel(child); - } + statement.forEachChild(this::travel); afterTravel(statement); return null; } @@ -1051,9 +853,7 @@ public Void visit(ForallStatement statement) { @Override public Void visit(IfStatement statement) { beforeTravel(statement); - for (var child : statement.children()) { - travel(child); - } + statement.forEachChild(this::travel); afterTravel(statement); return null; } @@ -1061,9 +861,7 @@ public Void visit(IfStatement statement) { @Override public Void visit(InstructionCallStatement statement) { beforeTravel(statement); - for (var child : statement.children()) { - travel(child); - } + statement.forEachChild(this::travel); afterTravel(statement); return null; } @@ -1071,9 +869,7 @@ public Void visit(InstructionCallStatement statement) { @Override public Void visit(LetStatement statement) { beforeTravel(statement); - for (var child : statement.children()) { - travel(child); - } + statement.forEachChild(this::travel); afterTravel(statement); return null; } @@ -1081,9 +877,7 @@ public Void visit(LetStatement statement) { @Override public Void visit(LockStatement statement) { beforeTravel(statement); - for (var child : statement.children()) { - travel(child); - } + statement.forEachChild(this::travel); afterTravel(statement); return null; } @@ -1091,9 +885,7 @@ public Void visit(LockStatement statement) { @Override public Void visit(MacroInstanceStatement statement) { beforeTravel(statement); - for (var child : statement.children()) { - travel(child); - } + statement.forEachChild(this::travel); afterTravel(statement); return null; } @@ -1101,9 +893,7 @@ public Void visit(MacroInstanceStatement statement) { @Override public Void visit(MacroMatchStatement statement) { beforeTravel(statement); - for (var child : statement.children()) { - travel(child); - } + statement.forEachChild(this::travel); afterTravel(statement); return null; } @@ -1111,9 +901,7 @@ public Void visit(MacroMatchStatement statement) { @Override public Void visit(MatchStatement statement) { beforeTravel(statement); - for (var child : statement.children()) { - travel(child); - } + statement.forEachChild(this::travel); afterTravel(statement); return null; } @@ -1121,9 +909,7 @@ public Void visit(MatchStatement statement) { @Override public Void visit(PlaceholderStatement statement) { beforeTravel(statement); - for (var child : statement.children()) { - travel(child); - } + statement.forEachChild(this::travel); afterTravel(statement); return null; } @@ -1131,9 +917,7 @@ public Void visit(PlaceholderStatement statement) { @Override public Void visit(RaiseStatement statement) { beforeTravel(statement); - for (var child : statement.children()) { - travel(child); - } + statement.forEachChild(this::travel); afterTravel(statement); return null; } @@ -1141,9 +925,7 @@ public Void visit(RaiseStatement statement) { @Override public Void visit(StatementList statement) { beforeTravel(statement); - for (var child : statement.children()) { - travel(child); - } + statement.forEachChild(this::travel); afterTravel(statement); return null; } diff --git a/vadl/main/vadl/ast/Definition.java b/vadl/main/vadl/ast/Definition.java index e4dd1e0a7..8b9cbe3d2 100644 --- a/vadl/main/vadl/ast/Definition.java +++ b/vadl/main/vadl/ast/Definition.java @@ -29,6 +29,7 @@ import java.util.Objects; import java.util.Set; import java.util.StringJoiner; +import java.util.function.Consumer; import java.util.stream.Collectors; import java.util.stream.Stream; import javax.annotation.Nullable; @@ -658,10 +659,11 @@ FormatField target() { } @Override - List children() { + void forEachChild(Consumer action) { // This has to be hardcoded here because for this format field it's a child but for some it's // the identfiyable name. - return List.of((Node) identifier, expr); + action.accept((Node) identifier); + action.accept(expr); } @Override @@ -725,10 +727,11 @@ FormatField target() { } @Override - List children() { + void forEachChild(Consumer action) { // This has to be hardcoded here because for this format field it's a child but for some it's // the identfiyable name. - return List.of((Node) identifier, expr); + action.accept((Node) identifier); + action.accept(expr); } @Override @@ -1493,10 +1496,10 @@ class PseudoInstructionDefinition extends InstructionSequenceDefinition } @Override - public List children() { + public void forEachChild(Consumer action) { // Since this class has no @Child annotations the Annotationprocessor doesn't find it. - return NodeChildrenRegistry.unsafeGetChildrenDirect(this, - (Class) getClass().getSuperclass()); + NodeChildrenRegistry.unsafeForEachChildDirect(this, + (Class) getClass().getSuperclass(), action); } @Override @@ -3589,10 +3592,10 @@ class AbiSequenceDefinition extends InstructionSequenceDefinition { } @Override - public List children() { + public void forEachChild(Consumer action) { // Since this class has no @Child annotations the Annotationprocessor doesn't find it. - return NodeChildrenRegistry.unsafeGetChildrenDirect(this, - (Class) getClass().getSuperclass()); + NodeChildrenRegistry.unsafeForEachChildDirect(this, + (Class) getClass().getSuperclass(), action); } @Override @@ -5500,12 +5503,15 @@ public AsmGrammarAlternativesDefinition(List> } @Override - List children() { + void forEachChild(Consumer action) { // This is too complicated for the @Child annotation - return alternatives.stream() - .flatMap(l -> l.stream().map(a -> (Node) a)) - .filter(Objects::nonNull) - .toList(); + for (var l : alternatives) { + for (var a : l) { + if (a != null) { + action.accept((Node) a); + } + } + } } @Override diff --git a/vadl/main/vadl/ast/Expr.java b/vadl/main/vadl/ast/Expr.java index 71ab7e920..c3a4afaf9 100644 --- a/vadl/main/vadl/ast/Expr.java +++ b/vadl/main/vadl/ast/Expr.java @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Objects; +import java.util.function.Consumer; import java.util.stream.Collectors; import javax.annotation.Nullable; import vadl.javaannotations.ast.Child; @@ -1825,16 +1826,15 @@ public List slices() { } @Override - List children() { + void forEachChild(Consumer action) { // This is too complicated for the @Child annotation - List childNodes = new ArrayList<>(); if (target != null) { - childNodes.add((Node) target); + action.accept((Node) target); } for (var a : argsIndices) { for (var v : a.values) { if (v != null) { - childNodes.add(v); + action.accept(v); } } } @@ -1842,12 +1842,11 @@ List children() { for (var a : subCall.argsIndices) { for (var v : a.values) { if (v != null) { - childNodes.add(v); + action.accept(v); } } } } - return childNodes; } void replaceArgsFor(int index, List newArgs) { @@ -2312,16 +2311,18 @@ class MatchExpr extends Expr { } @Override - List children() { + void forEachChild(Consumer action) { // This is too complicated for the @Child annotation - var childNodes = new ArrayList(); - childNodes.add(candidate); - cases.forEach(c -> { - childNodes.addAll(c.patterns); - childNodes.add(c.result); - }); - childNodes.add(defaultResult); - return childNodes.stream().filter(Objects::nonNull).toList(); + action.accept(candidate); + for (var c : cases) { + c.patterns.forEach(p -> action.accept(p)); + if (c.result != null) { + action.accept(c.result); + } + } + if (defaultResult != null) { + action.accept(defaultResult); + } } @Override @@ -2822,8 +2823,8 @@ R accept(ExprVisitor visitor) { } @Override - List children() { + void forEachChild(Consumer action) { // Remove this method when #293 is fixed. - return List.of(target); + action.accept(target); } } \ No newline at end of file diff --git a/vadl/main/vadl/ast/Statement.java b/vadl/main/vadl/ast/Statement.java index 717f8c40b..4d10c833c 100644 --- a/vadl/main/vadl/ast/Statement.java +++ b/vadl/main/vadl/ast/Statement.java @@ -20,6 +20,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Objects; +import java.util.function.Consumer; import javax.annotation.Nullable; import vadl.javaannotations.ast.Child; import vadl.types.TupleType; @@ -618,16 +619,18 @@ final class MatchStatement extends Statement { } @Override - List children() { + void forEachChild(Consumer action) { // This is too complicated for the @Child annotation - var childNodes = new ArrayList(); - childNodes.add(candidate); - cases.forEach(c -> { - childNodes.addAll(c.patterns); - childNodes.add(c.result); - }); - childNodes.add(defaultResult); - return childNodes.stream().filter(Objects::nonNull).toList(); + action.accept(candidate); + for (var c : cases) { + c.patterns.forEach(p -> action.accept(p)); + if (c.result != null) { + action.accept(c.result); + } + } + if (defaultResult != null) { + action.accept(defaultResult); + } } @Override diff --git a/vadl/main/vadl/ast/SymbolTable.java b/vadl/main/vadl/ast/SymbolTable.java index c6a24337c..7f5272118 100644 --- a/vadl/main/vadl/ast/SymbolTable.java +++ b/vadl/main/vadl/ast/SymbolTable.java @@ -749,7 +749,7 @@ public Void visit(AsmGrammarElementDefinition definition) { // Avoid creating a new scope since the elements should share the same scope beforeTravelWithoutScope(definition); definition.symbolTable = currentSymbols(); - definition.children().forEach(this::travel); + definition.forEachChild(this::travel); afterTravelWithoutScope(definition); return null; } @@ -762,9 +762,7 @@ public Void visit(AsmModifierDefinition definition) { // This isn't a identifyableNode so we need to add custom handling here. currentSymbols().defineSymbol(definition.stringLiteral.toString(), definition); - definition.children().stream() - .filter(c -> c != definition.stringLiteral) - .forEach(this::travel); + definition.forEachChild(c -> { if (c != definition.stringLiteral) travel(c); }); afterTravelWithoutScope(definition); return null; @@ -808,7 +806,7 @@ public Void visit(LetStatement statement) { statement.identifiers().forEach(identifier -> { childTable.defineSymbol(identifier.name, statement); }); - withSymbols(childTable, () -> statement.children().forEach(this::travel)); + withSymbols(childTable, () -> statement.forEachChild(this::travel)); afterTravel(statement); return null; @@ -838,7 +836,7 @@ public Void visit(ForallStatement statement) { @Override public Void visit(AliasDefinition definition) { beforeTravel(definition); - definition.children().forEach(this::travel); + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -853,7 +851,7 @@ public Void visit(LetExpr expr) { expr.identifiers().forEach(identifier -> { childTable.defineSymbol(identifier.name, expr); }); - withSymbols(childTable, () -> expr.children().forEach(this::travel)); + withSymbols(childTable, () -> expr.forEachChild(this::travel)); afterTravel(expr); return null; @@ -995,7 +993,7 @@ public Void visit(InstructionSetDefinition definition) { } } - definition.children().forEach(this::travel); + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -1012,7 +1010,7 @@ public Void visit(InstructionDefinition definition) { definition.formatNode = format; } - definition.children().forEach(this::travel); + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -1059,7 +1057,7 @@ public Void visit(AssemblyDefinition definition) { } } - definition.children().forEach(this::travel); + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -1121,7 +1119,7 @@ public Void visit(ApplicationBinaryInterfaceDefinition definition) { definition.symbolTable().extendBy(isa.symbolTable()); } - definition.children().forEach(this::travel); + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -1136,7 +1134,7 @@ public Void visit(MicroArchitectureDefinition definition) { definition.symbolTable().extendBy(isa.symbolTable()); } - definition.children().forEach(this::travel); + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -1160,7 +1158,7 @@ public Void visit(ProcessorDefinition definition) { } } - definition.children().forEach(this::travel); + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -1171,7 +1169,7 @@ public Void visit(CpuMemoryRegionDefinition def) { def.symbolTable().requireAs(def.memoryRef, MemoryDefinition.class); - def.children().forEach(this::travel); + def.forEachChild(this::travel); afterTravel(def); return null; } @@ -1253,7 +1251,7 @@ public Void visit(AsmDescriptionDefinition definition) { definition.symbolTable().extendBy(abi.symbolTable()); } - definition.children().forEach(this::travel); + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -1273,7 +1271,7 @@ public Void visit(AsmModifierDefinition definition) { .reportUnkownError("Relocation", relocation.pathToString(), relocation, suggestions); } - definition.children().forEach(this::travel); + definition.forEachChild(this::travel); afterTravel(definition); return null; } @@ -1312,9 +1310,7 @@ public Void visit(AsmGrammarElementDefinition definition) { } // All other children have the default handling - definition.children().stream() - .filter(c -> c != definition.attribute) - .forEach(this::travel); + definition.forEachChild(c -> { if (c != definition.attribute) travel(c); }); afterTravel(definition); return null; @@ -1357,9 +1353,7 @@ public Void visit(AsmGrammarLiteralDefinition definition) { // Resolve all other children like always // FIXME: At the moment id isn't even a child but I'm not sure if it should be so check in // later once we know it. - definition.children().stream() - .filter(c -> c != definition.id) - .forEach(this::travel); + definition.forEachChild(c -> { if (c != definition.id) travel(c); }); afterTravel(definition); From 4d10dbab6828cfb221c1d22957a55c000be1e33f Mon Sep 17 00:00:00 2001 From: flofriday Date: Wed, 13 May 2026 18:28:51 +0200 Subject: [PATCH 02/11] frontend: Reduce memory pressure of InterleavedTimingRecorder --- vadl-cli/main/vadl/cli/BaseCommand.java | 6 ++-- .../vadl/ast/InterleavedTimingRecorder.java | 29 +++++++++---------- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/vadl-cli/main/vadl/cli/BaseCommand.java b/vadl-cli/main/vadl/cli/BaseCommand.java index 089225358..9c7f56363 100644 --- a/vadl-cli/main/vadl/cli/BaseCommand.java +++ b/vadl-cli/main/vadl/cli/BaseCommand.java @@ -266,20 +266,20 @@ private Specification parseToVIAM() { var ast = parseToAst(); printSpecStats(ast, new DiskVirtualFileSystem()); printSpecStatsCsv(ast, new DiskVirtualFileSystem()); - ast.timingRecorder.passTimings.forEach( + ast.timingRecorder.passTimings.sequencedValues().forEach( t -> timings.add(new Timing(t.description(), t.durationNS()))); ast.timingRecorder.passTimings.clear(); dumpExpaned(ast); dumpUntyped(ast); TypeChecker.verify(ast); - ast.timingRecorder.passTimings.forEach( + ast.timingRecorder.passTimings.sequencedValues().forEach( t -> timings.add(new Timing(t.description(), t.durationNS()))); ast.timingRecorder.passTimings.clear(); dumpTyped(ast); var spec = ViamLowering.generate(ast); - ast.timingRecorder.passTimings.forEach( + ast.timingRecorder.passTimings.sequencedValues().forEach( t -> timings.add(new Timing(t.description(), t.durationNS()))); return spec; diff --git a/vadl/main/vadl/ast/InterleavedTimingRecorder.java b/vadl/main/vadl/ast/InterleavedTimingRecorder.java index e70a16ff6..060872345 100644 --- a/vadl/main/vadl/ast/InterleavedTimingRecorder.java +++ b/vadl/main/vadl/ast/InterleavedTimingRecorder.java @@ -17,9 +17,9 @@ package vadl.ast; import java.util.ArrayDeque; -import java.util.ArrayList; import java.util.Deque; -import java.util.List; +import java.util.LinkedHashMap; +import java.util.SequencedMap; import java.util.function.Supplier; /** @@ -52,27 +52,26 @@ public PassTiming(String description, long durationNS) { } } - public List passTimings = new ArrayList<>(); + public SequencedMap passTimings = new LinkedHashMap<>(); private Deque activeStack = new ArrayDeque<>(); private long activeStartTimeNS; private void addTiming(PassTiming passTiming) { - var existing = passTimings.stream() - .filter(timing -> timing.description.equals(passTiming.description)) - .findFirst(); - - existing.ifPresentOrElse(timing -> { - timing.durationNS += passTiming.durationNS; - }, () -> { - passTimings.add(passTiming); - }); + addTiming(passTiming.description, passTiming.durationNS); + } + + private void addTiming(String description, long durationNS) { + passTimings.compute(description, + (key, existingTiming) -> existingTiming == null ? new PassTiming(description, durationNS) : + new PassTiming(key, existingTiming.durationNS + durationNS) + ); } private void startRecording(String name) { // Add dummy timing to keep the order - addTiming(new PassTiming(name, 0)); + addTiming(name, 0); activeStack.push(name); continueRecording(); @@ -94,7 +93,7 @@ public void pauseRecording() { var name = activeStack.getFirst(); var consumedTimeNS = System.nanoTime() - activeStartTimeNS; activeStartTimeNS = -1; - addTiming(new PassTiming(name, consumedTimeNS)); + addTiming(name, consumedTimeNS); } private void endRecording() { @@ -147,7 +146,7 @@ public void withPassTiming(String name, Runnable pass) { } public void importAllTimings(InterleavedTimingRecorder other) { - other.passTimings.forEach(this::addTiming); + other.passTimings.values().forEach(this::addTiming); } } From 587fba7b77300a8b4e68ecd4c5ca0f41caf6eaa0 Mon Sep 17 00:00:00 2001 From: flofriday Date: Wed, 13 May 2026 19:41:23 +0200 Subject: [PATCH 03/11] frontend: Modify lists inplace for builtin collection loading --- vadl/main/vadl/ast/AstUtils.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/vadl/main/vadl/ast/AstUtils.java b/vadl/main/vadl/ast/AstUtils.java index 45477c94c..6313ed10a 100644 --- a/vadl/main/vadl/ast/AstUtils.java +++ b/vadl/main/vadl/ast/AstUtils.java @@ -77,9 +77,8 @@ static BuiltInTable.BuiltIn getOperatorBuiltIn(Operator operator, List arg } String finalOperatorSymbol = symbol; - var builtIns = operatorLookupTable.getOrDefault(finalOperatorSymbol, List.of()).stream() - .filter(b -> b.signature().argTypeClasses().size() == argTypes.size()) - .toList(); + var builtIns = operatorLookupTable.getOrDefault(finalOperatorSymbol, List.of()); + builtIns.removeIf(b -> b.signature().argTypeClasses().size() != argTypes.size()); // Sometimes there are a signed and unsigned version of builtin operation return switch (builtIns.size()) { From 860c6386fd9e910b818991588a4525f5241ca6ea Mon Sep 17 00:00:00 2001 From: flofriday Date: Wed, 13 May 2026 20:07:29 +0200 Subject: [PATCH 04/11] frontend: Avoid redundant allocations when creating passtimings --- vadl/main/vadl/ast/InterleavedTimingRecorder.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/vadl/main/vadl/ast/InterleavedTimingRecorder.java b/vadl/main/vadl/ast/InterleavedTimingRecorder.java index 060872345..7d14013c8 100644 --- a/vadl/main/vadl/ast/InterleavedTimingRecorder.java +++ b/vadl/main/vadl/ast/InterleavedTimingRecorder.java @@ -35,7 +35,7 @@ public class InterleavedTimingRecorder { * A pass timing of a single pass. */ public static class PassTiming { - private String description; + final private String description; private long durationNS; public String description() { @@ -64,8 +64,11 @@ private void addTiming(PassTiming passTiming) { private void addTiming(String description, long durationNS) { passTimings.compute(description, - (key, existingTiming) -> existingTiming == null ? new PassTiming(description, durationNS) : - new PassTiming(key, existingTiming.durationNS + durationNS) + (key, existingTiming) -> { + if (existingTiming == null) return new PassTiming(description, durationNS); + existingTiming.durationNS += durationNS; + return existingTiming; + } ); } From 359d1d3dfce218d41c6951d6b0a099abc7d5c660 Mon Sep 17 00:00:00 2001 From: flofriday Date: Wed, 13 May 2026 20:07:48 +0200 Subject: [PATCH 05/11] frontend: Add better syntathic node constructors for AST We often want to make copies of nodes (macro expanding) or create them syntactically. Some of the code reparsed the tokens again which wasn't needed. --- .../main/vadl/ast/AsmGrammarDefaultRules.java | 4 +- vadl/main/vadl/ast/Expr.java | 71 +++++++++++++------ vadl/main/vadl/ast/MacroExpander.java | 10 +-- vadl/main/vadl/ast/Statement.java | 8 ++- vadl/main/vadl/ast/TypeChecker.java | 3 +- 5 files changed, 67 insertions(+), 29 deletions(-) diff --git a/vadl/main/vadl/ast/AsmGrammarDefaultRules.java b/vadl/main/vadl/ast/AsmGrammarDefaultRules.java index a6ba4346c..c79a7a9c7 100644 --- a/vadl/main/vadl/ast/AsmGrammarDefaultRules.java +++ b/vadl/main/vadl/ast/AsmGrammarDefaultRules.java @@ -169,12 +169,12 @@ public static FunctionDefinition asmNegFunctionDefinition(WithLocation locatable new Parameter( new Identifier("x", loc), new TypeLiteral(new Identifier("SInt", loc), - List.of(new IntegerLiteral("64", loc)), + List.of(new IntegerLiteral(64, loc)), loc) ) )), new TypeLiteral(new Identifier("SInt", loc), - List.of(new IntegerLiteral("64", loc)), + List.of(new IntegerLiteral(64, loc)), loc), new UnaryExpr(new UnOp(UnaryOperator.NEGATIVE, loc), new Identifier("x", loc)), diff --git a/vadl/main/vadl/ast/Expr.java b/vadl/main/vadl/ast/Expr.java index c3a4afaf9..000bfa8ac 100644 --- a/vadl/main/vadl/ast/Expr.java +++ b/vadl/main/vadl/ast/Expr.java @@ -577,6 +577,25 @@ public IntegerLiteral(String token, SourceLocation loc) { this.loc = loc; } + // An alternative constructor for when the number is already known. + // Mostly used for macro expanding. + private IntegerLiteral(String token, BigInteger number, SourceLocation loc) { + this.token = token; + this.number = number; + this.loc = loc; + } + + // An internal constructor for when we want to create an integer literal synthetically + public IntegerLiteral(int number, SourceLocation loc) { + this.token = Integer.toString(number); + this.number = BigInteger.valueOf(number); + this.loc = loc; + } + + public IntegerLiteral copyWithLocation(SourceLocation location) { + return new IntegerLiteral(this.token, this.number, location); + } + @Override public SourceLocation location() { return loc; @@ -687,6 +706,17 @@ public BinaryLiteral(String token, SourceLocation loc) { } } + private BinaryLiteral(String token, BigInteger number, int bitWidth, SourceLocation location) { + this.token = token; + this.loc = location; + this.number = number; + this.bitWidth = bitWidth; + } + + public BinaryLiteral copyWithLocation(SourceLocation location) { + return new BinaryLiteral(this.token, this.number, this.bitWidth, location); + } + @Override public SourceLocation location() { return loc; @@ -804,17 +834,20 @@ public StringLiteral(String token, SourceLocation loc) { this.loc = loc; } - public StringLiteral(Identifier fromId, SourceLocation loc) { - // TODO More robust string escaping - only used for prettifying expanded AsStr code - this.token = '"' + fromId.name.replaceAll("\"", "\\\"") + '"'; - this.value = fromId.name; + public StringLiteral(String value) { + this.token = '"' + value + '"'; + this.value = value; + this.loc = SourceLocation.INVALID_SOURCE_LOCATION; + } + + public StringLiteral(String token, String value, SourceLocation loc) { + this.token = token; + this.value = value; this.loc = loc; } - public StringLiteral(String token) { - this.token = '"' + token + '"'; - this.value = token; - this.loc = SourceLocation.INVALID_SOURCE_LOCATION; + public StringLiteral copyWithLocation(SourceLocation location) { + return new StringLiteral(this.token, this.value, location); } @Override @@ -1403,18 +1436,15 @@ public TypeLiteral(Type type, SourceLocation loc) { } else if (type.getClass() == BitsType.class) { var bitsType = (BitsType) type; this.baseType = new Identifier("Bits", SourceLocation.INVALID_SOURCE_LOCATION); - this.sizeIndices = - List.of(new IntegerLiteral(Integer.toString(bitsType.bitWidth()), loc)); + this.sizeIndices = List.of(new IntegerLiteral(bitsType.bitWidth(), loc)); } else if (type.getClass() == UIntType.class) { var uintType = (UIntType) type; this.baseType = new Identifier("UInt", SourceLocation.INVALID_SOURCE_LOCATION); - this.sizeIndices = - List.of(new IntegerLiteral(Integer.toString(uintType.bitWidth()), loc)); + this.sizeIndices = List.of(new IntegerLiteral(uintType.bitWidth(), loc)); } else if (type.getClass() == SIntType.class) { var sintType = (SIntType) type; this.baseType = new Identifier("SInt", SourceLocation.INVALID_SOURCE_LOCATION); - this.sizeIndices = - List.of(new IntegerLiteral(Integer.toString(sintType.bitWidth()), loc)); + this.sizeIndices = List.of(new IntegerLiteral(sintType.bitWidth(), loc)); } else if (type.getClass() == FormatType.class) { var formatType = (FormatType) type; this.baseType = new Identifier(formatType.format.identifier().name, @@ -1423,9 +1453,10 @@ public TypeLiteral(Type type, SourceLocation loc) { } else if (type instanceof TensorType tensortype) { var innerLiteral = new TypeLiteral(tensortype.innerType(), loc); this.baseType = innerLiteral.baseType; - var indices = new ArrayList(); + var indices = + new ArrayList(tensortype.indexDims().size() + innerLiteral.sizeIndices.size()); for (var i : tensortype.indexDims()) { - indices.add(new IntegerLiteral(Integer.toString(i), loc)); + indices.add(new IntegerLiteral(i, loc)); } indices.addAll(innerLiteral.sizeIndices); this.sizeIndices = indices; @@ -1805,13 +1836,13 @@ public List args() { return argsIndices.isEmpty() ? List.of() : List.of(argsIndices.getFirst()); } else if (type instanceof TensorType tensorType && computedTarget() instanceof RegisterDefinition) { - return new ArrayList<>(argsIndices.subList(0, - Math.min(argsIndices.size(), tensorType.indexDims().size()))); + return argsIndices.subList(0, + Math.min(argsIndices.size(), tensorType.indexDims().size())); } else if (type instanceof TensorType tensorType && computedTarget() instanceof AliasDefinition aliasTarget && aliasTarget.kind == AliasDefinition.AliasKind.REGISTER) { - return new ArrayList<>(argsIndices.subList(0, - Math.min(argsIndices.size(), tensorType.indexDims().size()))); + return argsIndices.subList(0, + Math.min(argsIndices.size(), tensorType.indexDims().size())); } } return List.of(); diff --git a/vadl/main/vadl/ast/MacroExpander.java b/vadl/main/vadl/ast/MacroExpander.java index 176bd8076..b3fa56f59 100644 --- a/vadl/main/vadl/ast/MacroExpander.java +++ b/vadl/main/vadl/ast/MacroExpander.java @@ -245,7 +245,7 @@ public Expr visit(GroupedExpr expr) { @Override public Expr visit(IntegerLiteral expr) { - return new IntegerLiteral(expr.token, copyLoc(expr.loc)); + return expr.copyWithLocation(copyLoc(expr.location())); } @Override @@ -255,7 +255,7 @@ public Expr visit(WildcardLiteral expr) { @Override public Expr visit(BinaryLiteral expr) { - return new BinaryLiteral(expr.token, copyLoc(expr.loc)); + return expr.copyWithLocation(copyLoc(expr.loc)); } @Override @@ -265,7 +265,7 @@ public Expr visit(BoolLiteral expr) { @Override public Expr visit(StringLiteral expr) { - return new StringLiteral(expr.token, copyLoc(expr.loc)); + return expr.copyWithLocation(copyLoc(expr.loc)); } @Override @@ -474,7 +474,9 @@ public Expr visit(AsStrExpr expr) { } } - return new StringLiteral("\"" + nameBuilder + "\"", copyLoc(expr.location())); + var name = nameBuilder.toString(); + var token = "\"%s\"".formatted(name); + return new StringLiteral(token, name, copyLoc(expr.location())); } @Override diff --git a/vadl/main/vadl/ast/Statement.java b/vadl/main/vadl/ast/Statement.java index 4d10c833c..7f91f0ac7 100644 --- a/vadl/main/vadl/ast/Statement.java +++ b/vadl/main/vadl/ast/Statement.java @@ -152,7 +152,13 @@ List identifiers() { * @return the index/offset of the name provided. */ int getIndexOf(String name) { - return identifiers().stream().map(i -> i.name).toList().indexOf(name); + for (var idx = 0; idx < identifiers.size(); idx++) { + if (((Identifier)identifiers.get(idx)).name.equals(name)) { + return idx; + } + } + + return -1; } /** diff --git a/vadl/main/vadl/ast/TypeChecker.java b/vadl/main/vadl/ast/TypeChecker.java index 570d39298..72206604a 100644 --- a/vadl/main/vadl/ast/TypeChecker.java +++ b/vadl/main/vadl/ast/TypeChecker.java @@ -1803,8 +1803,7 @@ public Void visit(EnumerationDefinition definition) { } // if value is not set, we use the last value + 1. - entry.value = - new IntegerLiteral(String.valueOf(nextVal), SourceLocation.INVALID_SOURCE_LOCATION); + entry.value = new IntegerLiteral(nextVal, SourceLocation.INVALID_SOURCE_LOCATION); nextVal++; } From cc1bc8dd48e978d713b0e378c28b00da8589af2f Mon Sep 17 00:00:00 2001 From: flofriday Date: Wed, 13 May 2026 21:06:35 +0200 Subject: [PATCH 06/11] typechecker: Early exit annotation checking Most definitions don't have any annotations, so avoiding a lot of work early saves us 15MB here. --- vadl/main/vadl/ast/AstUtils.java | 19 +++++++++++++++++-- vadl/main/vadl/ast/BehaviorLowering.java | 22 ++++++++++++---------- vadl/main/vadl/ast/TypeChecker.java | 24 +++++++++++++----------- 3 files changed, 42 insertions(+), 23 deletions(-) diff --git a/vadl/main/vadl/ast/AstUtils.java b/vadl/main/vadl/ast/AstUtils.java index 6313ed10a..3fa9ae64a 100644 --- a/vadl/main/vadl/ast/AstUtils.java +++ b/vadl/main/vadl/ast/AstUtils.java @@ -18,6 +18,7 @@ import java.util.List; import java.util.Map; +import java.util.function.Consumer; import java.util.stream.Collectors; import javax.annotation.Nullable; import vadl.types.BuiltInTable; @@ -123,8 +124,22 @@ static BuiltInTable.BuiltIn getOperatorBuiltIn(Operator operator, List arg }; } - static List flatArguments(List args) { - return args.stream().flatMap(a -> a.values.stream()).collect(Collectors.toList()); + static List flatArguments(List argGroups) { + return argGroups.stream().flatMap(a -> a.values.stream()).collect(Collectors.toList()); + } + + static void forEachArgument(List argGroups, Consumer consumer) { + argGroups.forEach(a -> a.values.forEach(consumer)); + } + + static int argumentCount(List argGroups) { + int cnt = 0; + for (var args : argGroups) { + for (var arg: args.values) { + cnt++; + } + } + return cnt; } } diff --git a/vadl/main/vadl/ast/BehaviorLowering.java b/vadl/main/vadl/ast/BehaviorLowering.java index ef56de104..4d2db5bc3 100644 --- a/vadl/main/vadl/ast/BehaviorLowering.java +++ b/vadl/main/vadl/ast/BehaviorLowering.java @@ -1426,8 +1426,9 @@ public ExpressionNode visit(CallIndexExpr expr) { return visitStageCall(expr, stageDefinition); } - List argExprs = AstUtils.flatArguments(expr.args()); - var args = argExprs.stream().map(this::fetch).toList(); + var argGroups = expr.args(); + final var args = new NodeList(AstUtils.argumentCount(argGroups)); + AstUtils.forEachArgument(argGroups, arg -> args.add(this.fetch(arg))); var typeBeforeSlice = getViamType(expr.typeBeforeSlice()); ExpressionNode exprBeforeSlice; @@ -1435,21 +1436,21 @@ public ExpressionNode visit(CallIndexExpr expr) { // Builtin Call if (expr.computedBuiltIn != null) { if (BuiltInTable.ASM_PARSER_BUILT_INS.contains(expr.computedBuiltIn)) { - exprBeforeSlice = new AsmBuiltInCall(expr.computedBuiltIn, new NodeList<>(args), + exprBeforeSlice = new AsmBuiltInCall(expr.computedBuiltIn, args, typeBeforeSlice); } else { - exprBeforeSlice = new BuiltInCall(expr.computedBuiltIn, new NodeList<>(args), + exprBeforeSlice = new BuiltInCall(expr.computedBuiltIn, args, typeBeforeSlice); } } else { exprBeforeSlice = switch (expr.computedTarget()) { case FunctionDefinition funcDef -> new FuncCallNode( (Function) viamLowering.fetch(funcDef).orElseThrow(), - new NodeList<>(args), typeBeforeSlice); + args, typeBeforeSlice); case RelocationDefinition funcDef -> new FuncCallNode( (Function) viamLowering.fetch(funcDef).orElseThrow(), - new NodeList<>(args), typeBeforeSlice); + args, typeBeforeSlice); case RegisterDefinition regDef -> readTensorResourceConcatinated( (RegisterResource) viamLowering.fetch(regDef).orElseThrow(), args, @@ -1462,14 +1463,14 @@ public ExpressionNode visit(CallIndexExpr expr) { case REGISTER -> (ArtificialResource) lowered; case PROGRAM_COUNTER -> ((Counter) lowered).registerTensor(); }; - args = switch (aliasDef.kind) { + var aliasArgs = (NodeList) switch (aliasDef.kind) { case REGISTER -> args; case PROGRAM_COUNTER -> Stream.concat( ((Counter) lowered).indices().stream().map(ConstantNode::new), args.stream() ).toList(); }; - yield readTensorResourceConcatinated(resource, args, (DataType) typeBeforeSlice); + yield readTensorResourceConcatinated(resource, aliasArgs, (DataType) typeBeforeSlice); } case MemoryDefinition memDef -> { @@ -1747,8 +1748,9 @@ public SubgraphContext visit(AssignmentStatement statement) { throw new IllegalStateException("Unexpected target: " + statement); } - var argExprs = AstUtils.flatArguments(argGroups).stream().map(this::fetch) - .collect(Collectors.toCollection(NodeList::new)); + + var argExprs = new NodeList(AstUtils.argumentCount(argGroups)); + AstUtils.forEachArgument(argGroups, arg -> argExprs.add(this.fetch(arg))); var viamTargetDef = viamLowering.fetch(targetDef).orElseThrow(); var dynamicIndex = dynamicIndexExpr.get(); diff --git a/vadl/main/vadl/ast/TypeChecker.java b/vadl/main/vadl/ast/TypeChecker.java index 72206604a..c00703b25 100644 --- a/vadl/main/vadl/ast/TypeChecker.java +++ b/vadl/main/vadl/ast/TypeChecker.java @@ -241,6 +241,10 @@ void check(Statement stmt) { } private void verifyAnnotations(Definition def) { + if (def.annotations.isEmpty()) { + return; + } + // NOTE: This could have been done in the symbol resolver // Disallow the same annotation multiple times Map annotationNames = new HashMap<>(); @@ -2891,16 +2895,12 @@ public Void visit(SignalDefinition definition) { private void visitIdentifiable(Expr expr) { Node origin; String innerName; - String fullName; - + IsId isId = (IsId) expr; + origin = isId.target(); if (expr instanceof Identifier identifier) { - origin = identifier.target(); innerName = identifier.name; - fullName = identifier.name; } else if (expr instanceof IdentifierPath path) { - origin = path.target(); innerName = path.lastSegmentName(); - fullName = path.pathToString(); } else { throw new IllegalStateException("Unknown identifyable: " + expr.getClass().getSimpleName()); } @@ -3030,6 +3030,7 @@ private void visitIdentifiable(Expr expr) { // We might be here from a call expr and it might be necessary to handle the call for another // definition. + var fullName = isId.pathToString(); throw addErrorAndStopChecking(error("Invalid Expression", expr) .locationDescription(expr, "The name '%s' points to a `%s` which cannot be used as an expression.", fullName, @@ -3049,6 +3050,7 @@ private void visitIdentifiable(Expr expr) { return; } + var fullName = isId.pathToString(); throw new IllegalStateException( "Cannot find symbol `%s` found at: %s (The symbol resolver should already have caught that)" .formatted(fullName, expr.location().toConciseString())); @@ -3935,25 +3937,25 @@ private void processCallOfTarget(CallIndexExpr expr, Node callTarget) { } var argGroups = expr.args(); - var argExprs = AstUtils.flatArguments(argGroups); + var argCount = AstUtils.argumentCount(argGroups); - for (var argExpr : argExprs) { + AstUtils.forEachArgument(argGroups, argExpr -> { if (argExpr instanceof RangeExpr) { addErrorAndStopChecking(error("Invalid argument", argExpr) .locationNote(argExpr, "Expected argument value but got a range expression.") .build()); } - } + }); var type = typedNode.type(); if (type instanceof ConcreteRelationType relType && relType.argTypes().isEmpty()) { // if a relation type expects no arguments, no argument group is considered expr.typeBeforeSlice = relType.resultType(); } else if (type instanceof ConcreteRelationType relType) { - if (relType.argTypes().size() != argExprs.size()) { + if (relType.argTypes().size() != argCount) { addErrorAndStopChecking(error("Invalid number of arguments", expr) .locationNote(expr, "Expected %s arguments but got %s.", relType.argTypes().size(), - argExprs.size()).build()); + argCount).build()); } if (argGroups.size() != 1) { From 95d31bec9d389d17e89d233563585af647403ff6 Mon Sep 17 00:00:00 2001 From: flofriday Date: Wed, 13 May 2026 22:14:34 +0200 Subject: [PATCH 07/11] frontend: Remove redundant list creation in AnnotationTable --- vadl/main/vadl/ast/AnnotationTable.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/vadl/main/vadl/ast/AnnotationTable.java b/vadl/main/vadl/ast/AnnotationTable.java index a97928f4d..17ff01b3b 100644 --- a/vadl/main/vadl/ast/AnnotationTable.java +++ b/vadl/main/vadl/ast/AnnotationTable.java @@ -35,6 +35,7 @@ import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; +import java.util.stream.Stream; import javax.annotation.Nullable; import vadl.error.Diagnostic; import vadl.error.DiagnosticBuilder; @@ -418,12 +419,12 @@ static List availableAnnotationNames(Class klass) */ static Map> groupings( Definition definition) { - return groupings(definition.annotations.stream().map(d -> d.annotation).toList()); + return groupings(definition.annotations.stream().map(d -> d.annotation)); } private static Map> groupings( - List annotations) { - return annotations.stream().collect(Collectors.groupingBy(a -> a.groupProvider)); + Stream annotations) { + return annotations.collect(Collectors.groupingBy(a -> a.groupProvider)); } /** From 19175a77c870c1e0d3062f67cefaa8681afecf7a Mon Sep 17 00:00:00 2001 From: flofriday Date: Wed, 13 May 2026 22:36:46 +0200 Subject: [PATCH 08/11] lowering: Remove redundant location setting --- vadl/main/vadl/ast/AstUtils.java | 4 +--- .../vadl/ast/InterleavedTimingRecorder.java | 6 ++++-- vadl/main/vadl/ast/Statement.java | 2 +- vadl/main/vadl/ast/SymbolTable.java | 18 +++++++++++++++--- vadl/main/vadl/ast/ViamLowering.java | 3 --- 5 files changed, 21 insertions(+), 12 deletions(-) diff --git a/vadl/main/vadl/ast/AstUtils.java b/vadl/main/vadl/ast/AstUtils.java index 3fa9ae64a..531641dd4 100644 --- a/vadl/main/vadl/ast/AstUtils.java +++ b/vadl/main/vadl/ast/AstUtils.java @@ -135,9 +135,7 @@ static void forEachArgument(List argGroups, Consumer argGroups) { int cnt = 0; for (var args : argGroups) { - for (var arg: args.values) { - cnt++; - } + cnt += args.values.size(); } return cnt; } diff --git a/vadl/main/vadl/ast/InterleavedTimingRecorder.java b/vadl/main/vadl/ast/InterleavedTimingRecorder.java index 7d14013c8..aad5801d3 100644 --- a/vadl/main/vadl/ast/InterleavedTimingRecorder.java +++ b/vadl/main/vadl/ast/InterleavedTimingRecorder.java @@ -35,7 +35,7 @@ public class InterleavedTimingRecorder { * A pass timing of a single pass. */ public static class PassTiming { - final private String description; + private final String description; private long durationNS; public String description() { @@ -65,7 +65,9 @@ private void addTiming(PassTiming passTiming) { private void addTiming(String description, long durationNS) { passTimings.compute(description, (key, existingTiming) -> { - if (existingTiming == null) return new PassTiming(description, durationNS); + if (existingTiming == null) { + return new PassTiming(description, durationNS); + } existingTiming.durationNS += durationNS; return existingTiming; } diff --git a/vadl/main/vadl/ast/Statement.java b/vadl/main/vadl/ast/Statement.java index 7f91f0ac7..79092fce1 100644 --- a/vadl/main/vadl/ast/Statement.java +++ b/vadl/main/vadl/ast/Statement.java @@ -153,7 +153,7 @@ List identifiers() { */ int getIndexOf(String name) { for (var idx = 0; idx < identifiers.size(); idx++) { - if (((Identifier)identifiers.get(idx)).name.equals(name)) { + if (((Identifier) identifiers.get(idx)).name.equals(name)) { return idx; } } diff --git a/vadl/main/vadl/ast/SymbolTable.java b/vadl/main/vadl/ast/SymbolTable.java index 7f5272118..13f68ceae 100644 --- a/vadl/main/vadl/ast/SymbolTable.java +++ b/vadl/main/vadl/ast/SymbolTable.java @@ -762,7 +762,11 @@ public Void visit(AsmModifierDefinition definition) { // This isn't a identifyableNode so we need to add custom handling here. currentSymbols().defineSymbol(definition.stringLiteral.toString(), definition); - definition.forEachChild(c -> { if (c != definition.stringLiteral) travel(c); }); + definition.forEachChild(c -> { + if (c != definition.stringLiteral) { + travel(c); + } + }); afterTravelWithoutScope(definition); return null; @@ -1310,7 +1314,11 @@ public Void visit(AsmGrammarElementDefinition definition) { } // All other children have the default handling - definition.forEachChild(c -> { if (c != definition.attribute) travel(c); }); + definition.forEachChild(c -> { + if (c != definition.attribute) { + travel(c); + } + }); afterTravel(definition); return null; @@ -1353,7 +1361,11 @@ public Void visit(AsmGrammarLiteralDefinition definition) { // Resolve all other children like always // FIXME: At the moment id isn't even a child but I'm not sure if it should be so check in // later once we know it. - definition.forEachChild(c -> { if (c != definition.id) travel(c); }); + definition.forEachChild(c -> { + if (c != definition.id) { + travel(c); + } + }); afterTravel(definition); diff --git a/vadl/main/vadl/ast/ViamLowering.java b/vadl/main/vadl/ast/ViamLowering.java index 36be14ebf..17b5c8b5b 100644 --- a/vadl/main/vadl/ast/ViamLowering.java +++ b/vadl/main/vadl/ast/ViamLowering.java @@ -302,7 +302,6 @@ Optional fetch(Optional definition) Optional fetch(FormatField field) { // FIXME: Try to evaluate the format if it hasn't been seen before. var result = Optional.ofNullable(formatFieldCache.get(field)); - result.ifPresent(f -> f.setSourceLocationIfNotSet(field.location())); return result; } @@ -315,7 +314,6 @@ Optional fetch(FormatField field) { Optional fetch(Parameter parameter) { // FIXME: Try to evaluate the format if it hasn't been seen before. var result = Optional.ofNullable(parameterCache.get(parameter)); - result.ifPresent(f -> f.setSourceLocationIfNotSet(parameter.location())); return result; } @@ -1323,7 +1321,6 @@ private Format.FieldEncoding getFieldEncoding(vadl.viam.Identifier ident, var behavior = new BehaviorLowering(this).getFunctionGraph(encode.expr, ident.toString()); var field = (Format.Field) requireNonNull(formatFieldCache.get(encode.target())); var encoding = new Format.FieldEncoding(ident, field, behavior); - encoding.setSourceLocation(encode.location()); checkNoResourceAccesses(behavior, "field access encoding"); checkLeafNodes(behavior, (n) -> { From 87cd63da948180b81861543286b090f3f4c94b39 Mon Sep 17 00:00:00 2001 From: flofriday Date: Thu, 14 May 2026 14:31:41 +0200 Subject: [PATCH 09/11] frontend: Refactor symbol table --- vadl/main/vadl/ast/BehaviorLowering.java | 2 +- vadl/main/vadl/ast/ParserUtils.java | 1 + vadl/main/vadl/ast/SpecStatAnalyser.java | 2 +- vadl/main/vadl/ast/SymbolTable.java | 379 ++++++++++++----------- vadl/main/vadl/ast/TypeChecker.java | 24 +- vadl/main/vadl/ast/vadl.ATG | 3 +- 6 files changed, 201 insertions(+), 210 deletions(-) diff --git a/vadl/main/vadl/ast/BehaviorLowering.java b/vadl/main/vadl/ast/BehaviorLowering.java index 4d2db5bc3..ee342aeda 100644 --- a/vadl/main/vadl/ast/BehaviorLowering.java +++ b/vadl/main/vadl/ast/BehaviorLowering.java @@ -1468,7 +1468,7 @@ public ExpressionNode visit(CallIndexExpr expr) { case PROGRAM_COUNTER -> Stream.concat( ((Counter) lowered).indices().stream().map(ConstantNode::new), args.stream() - ).toList(); + ).collect(Collectors.toCollection(NodeList::new)); }; yield readTensorResourceConcatinated(resource, aliasArgs, (DataType) typeBeforeSlice); } diff --git a/vadl/main/vadl/ast/ParserUtils.java b/vadl/main/vadl/ast/ParserUtils.java index 0a7eef71e..1aeee5aff 100644 --- a/vadl/main/vadl/ast/ParserUtils.java +++ b/vadl/main/vadl/ast/ParserUtils.java @@ -686,6 +686,7 @@ static Node expandNode(Parser parser, Node node) { * @return returns the original isaDefs with the Assemblies expanded. */ static List expandAssemblyDefinitionsInIsa(List isaDefs) { + // FIXME: We could modify the list inplace here for performance return isaDefs.stream() .flatMap(def -> { if (def instanceof AssemblyDefinition assembly) { diff --git a/vadl/main/vadl/ast/SpecStatAnalyser.java b/vadl/main/vadl/ast/SpecStatAnalyser.java index 1c92f71e5..09d66d23e 100644 --- a/vadl/main/vadl/ast/SpecStatAnalyser.java +++ b/vadl/main/vadl/ast/SpecStatAnalyser.java @@ -133,7 +133,7 @@ private boolean ifNewUnexpanded(WithLocation loctable, Runnable action) { */ private void addModelNodes(SymbolTable table) { table.macroSymbols.values().forEach(symbol -> { - var node = symbol.origin(); + var node = symbol; ifNewUnexpanded(node, () -> { if (node instanceof ModelDefinition) { modelDefinitions++; diff --git a/vadl/main/vadl/ast/SymbolTable.java b/vadl/main/vadl/ast/SymbolTable.java index 13f68ceae..fda3b42b7 100644 --- a/vadl/main/vadl/ast/SymbolTable.java +++ b/vadl/main/vadl/ast/SymbolTable.java @@ -28,10 +28,9 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.function.Supplier; +import java.util.stream.Collectors; import javax.annotation.Nullable; import vadl.error.Diagnostic; -import vadl.error.DiagnosticBuilder; import vadl.types.BuiltInTable; import vadl.types.Type; import vadl.types.asmTypes.AsmType; @@ -42,48 +41,42 @@ class SymbolTable { @Nullable SymbolTable parent; - final Map symbols = new HashMap<>(); - final Map macroSymbols = new HashMap<>(); + + final Map symbols = new HashMap<>(); + final Map macroSymbols = new HashMap<>(); // the errors list is the same obj as the parent's error list List errors; - public SymbolTable() { - parent = null; - errors = new ArrayList<>(); - } - - private SymbolTable(SymbolTable parent, List errors) { - this.parent = parent; - this.errors = errors; - } - - sealed interface Symbol { - } - - record AstSymbol(Node origin) implements Symbol { - } - - record BuiltInSymbol() implements Symbol { - } - /** * Load all builtin function names into the scope so that the names are reserved and can be used * (in some contextes). However, we don't store any useful informations with them and the next * passes (the type-checker) needs to know on it's own how to deal with them. */ - void loadBuiltins() { - // Load all "real" builtins - BuiltInTable.builtIns().map(BuiltInTable.BuiltIn::name) - .forEach(name -> symbols.put(name, new BuiltInSymbol())); + static HashSet builtinNames; + + static { + builtinNames = BuiltInTable.builtIns() + .map(BuiltInTable.BuiltIn::name) + .collect(Collectors.toCollection(HashSet::new)); // Add pseudo buildins - symbols.put("VADL::mod", new BuiltInSymbol()); - symbols.put("VADL::div", new BuiltInSymbol()); - symbols.put("start", new BuiltInSymbol()); - symbols.put("executable", new BuiltInSymbol()); - symbols.put("halt", new BuiltInSymbol()); + builtinNames.add("VADL::mod"); + builtinNames.add("VADL::div"); + builtinNames.add("start"); + builtinNames.add("executable"); + builtinNames.add("halt"); // TODO: Remove this once migration of decimal to sdec is done for all specs #409 - symbols.put("decimal", new BuiltInSymbol()); + builtinNames.add("decimal"); + } + + public SymbolTable() { + parent = null; + errors = new ArrayList<>(); + } + + private SymbolTable(SymbolTable parent, List errors) { + this.parent = parent; + this.errors = errors; } /** @@ -139,184 +132,221 @@ SymbolTable pop() { return parent; } + /** + * Check if the identifier resolves to a builtin. + * + * @param isId to be resolved + * @return true if the identifier resolves to a builtin, false otherwise + */ + boolean isBuiltin(IsId isId) { + var name = isId.pathToString(); + return builtinNames.contains(name); + } + /** + * Define a symbol or macro symbol from a user definition. + * + * @param name of the new symbol. + * @param origin of the symbol definition. + */ void defineSymbol(String name, Node origin) { if (origin instanceof ModelDefinition || origin instanceof ModelTypeDefinition || origin instanceof RecordTypeDefinition) { verifyMacroAvailable(name, origin); - macroSymbols.put(name, new AstSymbol(origin)); + macroSymbols.put(name, origin); } else { verifyAvailable(name, origin); - symbols.put(name, new AstSymbol(origin)); + symbols.put(name, origin); } } + /** + * A wrapper for {@link #defineSymbol(String, Node)} for {@link IdentifiableNode}. + * + * @param origin to be added to the scope, name will be automatically infered. + */ void defineSymbol(T origin) { var name = origin.identifier().name; defineSymbol(name, origin); } + /** + * A wrapper for {@link #defineSymbol(String, Node)} for {@link ModelDefinition}. + * + * @param modelDefinition to be added to the scope, name will be automatically infered. + */ void addModelDefinition(ModelDefinition modelDefinition) { // Note: We cannot use .identifier here because the identifier might not be initialized with // macros that generate macros. defineSymbol(modelDefinition.toMacro().name().name, modelDefinition); } + /** + * Internal use only. + * Finds a symbol in the current scope. + * + * @param name of the symbol to resolve + * @return the symbol if found, null otherwise + */ @Nullable - private Symbol resolveSymbol(String name) { + private Node resolveName(String name) { var symbol = symbols.get(name); - if (symbol != null) { return symbol; } if (parent != null) { - return parent.resolveSymbol(name); + return parent.resolveName(name); } return null; } + /** + * Internal use only. + * Finds a symbol in the current scope by path + * + * @param path to the symbol to resolve + * @return the symbol if found, null otherwise + */ @Nullable - private Symbol resolveBuiltinSymbol(String name) { - var root = this; - while (root.parent != null) { - root = root.parent; - } - - // FIXME: I don't think the namespace prefix should be in here - var symbol = root.resolveSymbol(name); - if (symbol == null) { - symbol = root.resolveSymbol("VADL::" + name); - } - - if (symbol instanceof BuiltInSymbol) { - return symbol; - } - return null; - } - - @Nullable - private Symbol resolveSymbolPath(List path) { - // The vadl namespace is a pseudo namespace and points to the root and its buitlin functions - if (path.size() == 2 && path.get(0).equalsIgnoreCase("vadl")) { - return resolveBuiltinSymbol(path.get(1)); - } - + private Node resolvePath(List path) { if (path.size() == 1) { return symbols.get(path.get(0)); } - var namespace = (AstSymbol) resolveSymbol(path.get(0)); + var namespace = resolveName(path.get(0)); if (namespace == null) { return null; } - return namespace.origin.symbolTable().resolveSymbolPath(path.subList(1, path.size())); - } - - @Nullable - private Symbol resolve(Identifier ident) { - var symbol = resolveSymbol(ident.name); - if (symbol instanceof AstSymbol(Node origin)) { - ident.target = origin; - } - return symbol; - } - - @Nullable - private Symbol resolve(IdentifierPath path) { - var symbol = resolveSymbolPath(path.pathToSegments()); - if (symbol instanceof AstSymbol(Node origin)) { - path.target = origin; - } - return symbol; + return namespace.symbolTable().resolvePath(path.subList(1, path.size())); } + /** + * Internal use only. + * Finds an Identifier or IdentifierPath in the current scope or by path + * + * @param id to resolve + * @return the symbol if found, null otherwise + */ @Nullable - private Symbol resolve(IsId id) { + private Node find(IsId id) { return switch (id) { - case Identifier ident -> resolve(ident); - case IdentifierPath path -> resolve(path); + case Identifier ident -> { + var symbol = resolveName(ident.name); + if (symbol != null) { + ident.target = symbol; + } + yield symbol; + } + case IdentifierPath path -> { + var symbol = resolvePath(path.pathToSegments()); + if (symbol != null) { + path.target = symbol; + } + yield symbol; + } default -> throw new IllegalArgumentException("Illegal identifier type: " + id.getClass()); }; } + /** + * Internal use only. + * Finds a macro symbol by name in the current scope or by path + * + * @param name to resolve + * @return the origin if it could be found, null otherwise + */ @Nullable - private Node resolveMacroSymbol(String name) { + private Node resolveMacroName(String name) { var symbol = macroSymbols.get(name); if (symbol == null && parent != null) { - return parent.resolveMacroSymbol(name); + return parent.resolveMacroName(name); } if (symbol == null) { return null; } - return symbol.origin; + return symbol; } - @Nullable - Macro getMacro(String name) { - var origin = resolveMacroSymbol(name); - if (origin instanceof ModelDefinition macro) { - return macro.toMacro(); - } - - return null; + /** + * Finds a macro symbol by name in the current scope. + * + * @param usage the identifier that should be resolved + * @param type of the node to be resolved + * @return the origin if it could be found, null otherwise + */ + public @Nullable T findAs(IsId usage, Class type) { + var symbol = find(usage); + return type.isInstance(symbol) ? type.cast(symbol) : null; } + ; - @Nullable T findAs(IsId usage, Class type) { - var symbol = resolve(usage); - return switch (symbol) { - case null -> null; - case AstSymbol astSymbol -> - type.isInstance(astSymbol.origin) ? type.cast(astSymbol.origin) : null; - case BuiltInSymbol ignored -> null; - }; + + /** + * Finds a macro symbol by name in the current scope or by name. + * + * @param name of the macro, syntax type or record type. + * @param type of to be searched + * @return the found origin or null + */ + @Nullable + public T findMacroAs(String name, Class type) { + var origin = resolveMacroName(name); + return type.isInstance(origin) ? type.cast(origin) : null; + } + + /** + * Short cut to get the macro of a modeldefinition. + * + * @param name to be resolved. + * @return the Macro if it exists otherwise null. + */ + @Nullable + Macro getMacro(String name) { + var model = findMacroAs(name, ModelDefinition.class); + if (model == null) { + return null; + } + return model.toMacro(); } /** - * This will add an error to {@link #errors} if the identifier couldn't be resolved - * to a node of the given type. - * In this case it will return null, so the user must check the result before continuing. + * Resolve the provided identifier or report an error. * * @param usage the identifier that should be resolved * @param type the type that the resolved node must have * @return the resolved node, or null if it could not be resolved with the given type */ - // FIXME: I don't like how it's called require but still returns null @Nullable T requireAs(IsId usage, Class type) { var origin = findAs(usage, type); if (origin != null) { return origin; } + // FIXME: Add a custom message if the object was found but not the specified type. var suggestions = Levenshtein.suggestions(usage.pathToString(), allSymbolNamesOf(type)); reportUnkownError(Node.nodeNameFor(type), usage.pathToString(), usage, suggestions); return null; } /** - * Finds the symbol for a given {@link IsId} and throws the error provided by the error - * builder if the symbol was not found. + * Resolve the provided identifier or report an error. + * + * @param usage to be resolved. */ - Symbol requireSymbol(IsId name, Supplier errorBuilder) { - var symbol = resolve(name); - if (symbol == null) { - throw errorBuilder.get().build(); + void requireAny(IsId usage) { + var origin = find(usage); + if (origin != null || isBuiltin(usage)) { + return; } - return symbol; - } - /** - * This allows the {@link Parser} to find an ISA during parsing. - * This is only possible for ISA definitions and only used by the parser. - */ - @Nullable - InstructionSetDefinition requireIsaDef(IsId usage) { - return requireAs(usage, InstructionSetDefinition.class); + var suggestions = Levenshtein.suggestions(usage.pathToString(), allSymbolNames()); + reportUnkownError("Symbol", usage.pathToString(), usage, suggestions); } /** @@ -353,7 +383,7 @@ FormatDefinition findInstructionFormat(Identifier instrId) { * @return the syntax type it refers to */ SyntaxType requireSyntaxType(Identifier identifier) { - var symbol = resolveMacroSymbol(identifier.name); + var symbol = resolveMacroName(identifier.name); if (symbol instanceof RecordTypeDefinition recordType) { return recordType.recordType; } else if (symbol instanceof ModelTypeDefinition modelType) { @@ -366,17 +396,30 @@ SyntaxType requireSyntaxType(Identifier identifier) { throw ParserUtils.unknownSyntaxTypeError(identifier.name, this, identifier.location()); } + /** + * Internal use only. + */ + private void collectAllSymbolNamesOf(Set collector, Class... classes) { + symbols.entrySet().stream() + .filter(entry -> entry.getValue() != null + && Arrays.stream(classes).anyMatch(klass -> klass.isInstance(entry.getValue()))) + .map(Map.Entry::getKey) + .forEach(collector::add); + + if (parent != null) { + parent.collectAllSymbolNamesOf(collector, classes); + } + } + /** * Returns all symbol names in scope. * * @return the set of all available names. */ Set allSymbolNames() { - var names = new HashSet<>(symbols.keySet()); - if (parent != null) { - names.addAll(parent.allSymbolNames()); - } - return names; + var symbols = new HashSet<>(builtinNames); + collectAllSymbolNamesOf(symbols, Node.class); + return symbols; } /** @@ -387,17 +430,9 @@ Set allSymbolNames() { */ @SafeVarargs final Set allSymbolNamesOf(Class... classes) { - var matchingNames = symbols.entrySet().stream() - .filter(entry -> entry.getValue() instanceof AstSymbol astSymbol - && Arrays.stream(classes).anyMatch(klass -> klass.isInstance(astSymbol.origin))) - .map(Map.Entry::getKey) - .toList(); - - var names = new HashSet<>(matchingNames); - if (parent != null) { - names.addAll(parent.allSymbolNamesOf(classes)); - } - return names; + var symbols = new HashSet(); + collectAllSymbolNamesOf(symbols, classes); + return symbols; } /** @@ -410,7 +445,7 @@ final Set allSymbolNamesOf(Class... classes) { final Set allMacroSymbolNamesOf(Class... classes) { var matchingNames = macroSymbols.entrySet().stream() .filter(entry -> Arrays.stream(classes) - .anyMatch(klass -> klass.isInstance(entry.getValue().origin))) + .anyMatch(klass -> klass.isInstance(entry.getValue()))) .map(Map.Entry::getKey) .toList(); @@ -431,17 +466,14 @@ void extendBy(SymbolTable other) { for (var entry : other.symbols.entrySet()) { var name = entry.getKey(); var symbol = entry.getValue(); - switch (symbol) { - case AstSymbol astSymbol -> defineSymbol(name, astSymbol.origin); - case BuiltInSymbol ignored -> { /* do nothing, already defined */ } - } + defineSymbol(name, symbol); } // add macro symbols to this symbol table. // #defineSymbol will correctly assign symbol to macroSymbols for (var entry : other.macroSymbols.entrySet()) { var name = entry.getKey(); - AstSymbol symbol = entry.getValue(); - defineSymbol(name, symbol.origin); + var symbol = entry.getValue(); + defineSymbol(name, symbol); } } @@ -469,8 +501,7 @@ private void verifyAvailable(String name, Node origin) { } var otherSymbol = symbols.get(name); - if (otherSymbol instanceof AstSymbol astSymbol - && astSymbol.origin == origin) { + if (otherSymbol == origin) { // if the other origin is the same node, the "redefinition" is ok. // this can happen when we have a diamond pattern like isa0 -> abi -> superisa // and isa0 -> superisa. @@ -483,12 +514,8 @@ private void verifyAvailable(String name, Node origin) { .locationDescription(originLoc, "Second definition here.") .note("All symbols must have a unique name."); - - if (otherSymbol instanceof BuiltInSymbol) { - error.description("`%s` is a builtin and cannot be used as a name", name); - } else if (otherSymbol instanceof AstSymbol astSymbol) { - var other = astSymbol.origin; - var otherLoc = getIdentifierLocation(other); + if (otherSymbol != null) { + var otherLoc = getIdentifierLocation(otherSymbol); error.locationDescription(otherLoc, "First defined here."); } @@ -500,7 +527,7 @@ private void verifyMacroAvailable(String name, Node origin) { return; } - var other = macroSymbols.get(name).origin(); + var other = macroSymbols.get(name); if (other == origin) { // if the other origin is the same node, the "redefinition" is ok. // this can happen when we have a diamond pattern like isa0 -> abi -> superisa @@ -528,8 +555,7 @@ private void reportUnkownError(String type, String actual, WithLocation locatabl ); if (suggestions != null && !suggestions.isEmpty()) { - diagnostic = - diagnostic.suggestions(suggestions); + diagnostic = diagnostic.suggestions(suggestions); } errors.add(diagnostic.build()); @@ -905,27 +931,13 @@ private SymbolResolver() { @Override public Void visit(Identifier expr) { - var symbol = expr.symbolTable().resolve(expr); - if (symbol == null) { - var suggestions = - Levenshtein.suggestions(expr.pathToString(), expr.symbolTable().allSymbolNames()); - - expr.symbolTable() - .reportUnkownError("Symbol", expr.pathToString(), expr.location(), suggestions); - } + expr.symbolTable().requireAny(expr); return null; } @Override public Void visit(IdentifierPath expr) { - var symbol = expr.symbolTable().resolve(expr); - if (symbol == null) { - var suggestions = - Levenshtein.suggestions(expr.pathToString(), expr.symbolTable().allSymbolNames()); - - expr.symbolTable() - .reportUnkownError("Symbol", expr.pathToString(), expr.location(), suggestions); - } + expr.symbolTable().requireAny(expr); return null; } @@ -962,7 +974,7 @@ public Void visit(AnnotationDefinition definition) { try { definition.annotation.resolveName(definition, this); } catch (Diagnostic d) { - requireNonNull(definition.symbolTable).errors.add(d); + definition.symbolTable().errors.add(d); } return null; } @@ -976,7 +988,7 @@ public Void visit(TypeLiteral expr) { // Because the typechecker does the real resolution, checking and error reporting here, but // it is nice for the lsp to already have some resolution if we can get to it easily. if (expr.symbolTable().findAs(expr.baseType, Definition.class) != null) { - requireNonNull(expr.symbolTable).resolve(expr.baseType); + expr.symbolTable().find(expr.baseType); } expr.sizeIndices.forEach(index -> index.accept(this)); @@ -1265,16 +1277,7 @@ public Void visit(AsmModifierDefinition definition) { beforeTravel(definition); var relocation = definition.relocation; - var symbol = definition.symbolTable().resolve(relocation); - if (symbol == null) { - var suggestions = Levenshtein.suggestions( - relocation.name, - definition.symbolTable().allSymbolNames()); - - definition.symbolTable() - .reportUnkownError("Relocation", relocation.pathToString(), relocation, suggestions); - } - + definition.symbolTable().requireAs(relocation, RelocationDefinition.class); definition.forEachChild(this::travel); afterTravel(definition); return null; @@ -1345,7 +1348,7 @@ public Void visit(AsmGrammarLiteralDefinition definition) { // Id needs special treatment if (definition.id != null) { - var idSymbol = definition.symbolTable().resolve(definition.id); + var idSymbol = definition.symbolTable().find(definition.id); if (idSymbol == null) { var suggestions = Levenshtein.suggestions( definition.id.name, diff --git a/vadl/main/vadl/ast/TypeChecker.java b/vadl/main/vadl/ast/TypeChecker.java index c00703b25..d55e0f899 100644 --- a/vadl/main/vadl/ast/TypeChecker.java +++ b/vadl/main/vadl/ast/TypeChecker.java @@ -3825,30 +3825,18 @@ private void visitSubCall(CallIndexExpr expr, Type typeBeforeSubCall) { @Override public Void visit(CallIndexExpr expr) { - // try to find target symbol in symbol table. - // as identifiers only store AstSymbol origins, and the call expr might refer to a - // BuiltInSymbol, we must do a separate request to the symbol table an can't rely on the - // expression's identifier target. - var targetSymbol = expr.symbolTable().requireSymbol(expr.target.path(), () -> { - var suggestions = Levenshtein.suggestions(expr.target.path().pathToString(), - expr.symbolTable().allSymbolNames()); - - return error("Unknown call target", expr.target) - .locationNote(expr.target, "Nothing found that can be called with this name.") - .suggestions(suggestions); - }); - + var target = expr.target.path().target(); // A hack for stage definitions since they don't fit into our typesystem - if (targetSymbol instanceof SymbolTable.AstSymbol astSymbol - && astSymbol.origin() instanceof StageDefinition stageDef) { + if (target instanceof StageDefinition stageDef) { processStageCall(expr, stageDef); return null; } - switch (targetSymbol) { - case SymbolTable.AstSymbol astSymbol -> processCallOfTarget(expr, astSymbol.origin()); - case SymbolTable.BuiltInSymbol ignored -> processCallOfBuiltIn(expr); + if (target != null) { + processCallOfTarget(expr, target); + } else { + processCallOfBuiltIn(expr); } var slices = expr.slices(); diff --git a/vadl/main/vadl/ast/vadl.ATG b/vadl/main/vadl/ast/vadl.ATG index 725fa0b19..780e294fb 100644 --- a/vadl/main/vadl/ast/vadl.ATG +++ b/vadl/main/vadl/ast/vadl.ATG @@ -42,7 +42,6 @@ COMPILER vadl { ast.rootSymbolTable = new SymbolTable(); - ast.rootSymbolTable.loadBuiltins(); } SourceLocation lastTokenLoc() { @@ -436,7 +435,7 @@ The #commonDefinitionList production rule lists (beside macro definitions) all c identifierOrPlaceholder (. extending.add(isa); .) } ] (. extending.forEach(e -> { - var extendingIsa = macroTable.requireIsaDef(e); + var extendingIsa = macroTable.requireAs(e, InstructionSetDefinition.class); if (extendingIsa != null) readMacroSymbols(macroTable, extendingIsa); }); .) From 01f9072d56d9df1919b04ab31cbf2a152ac77ef8 Mon Sep 17 00:00:00 2001 From: flofriday Date: Thu, 14 May 2026 21:53:20 +0200 Subject: [PATCH 10/11] frontend: Optimize assembly definition expansion --- vadl/main/vadl/ast/MacroExpander.java | 2 +- vadl/main/vadl/ast/Parser.frame | 1 + vadl/main/vadl/ast/ParserUtils.java | 29 ++++++++++++++++----------- vadl/main/vadl/ast/vadl.ATG | 5 +++-- 4 files changed, 22 insertions(+), 15 deletions(-) diff --git a/vadl/main/vadl/ast/MacroExpander.java b/vadl/main/vadl/ast/MacroExpander.java index b3fa56f59..c3711db18 100644 --- a/vadl/main/vadl/ast/MacroExpander.java +++ b/vadl/main/vadl/ast/MacroExpander.java @@ -215,7 +215,7 @@ public List expandAssemblies(AssemblyDefinition definition) result.add(new AssemblyDefinition( new ArrayList<>(List.of(identifier)), expandExpr(definition.expr), - copyLoc(definition.loc) + definition.loc )); } return result; diff --git a/vadl/main/vadl/ast/Parser.frame b/vadl/main/vadl/ast/Parser.frame index 8f58e46df..01ac91ddc 100644 --- a/vadl/main/vadl/ast/Parser.frame +++ b/vadl/main/vadl/ast/Parser.frame @@ -32,6 +32,7 @@ import vadl.utils.VirtualFileSystem; import vadl.error.Diagnostic; import static vadl.error.Diagnostic.error; +@SuppressWarnings("NonApiType") // We know what we are doing, the implementation matters sometimes. public class Parser { /** diff --git a/vadl/main/vadl/ast/ParserUtils.java b/vadl/main/vadl/ast/ParserUtils.java index 1aeee5aff..884588e77 100644 --- a/vadl/main/vadl/ast/ParserUtils.java +++ b/vadl/main/vadl/ast/ParserUtils.java @@ -25,12 +25,12 @@ import java.util.Arrays; import java.util.HashMap; import java.util.Iterator; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.regex.Pattern; import java.util.stream.Collectors; -import java.util.stream.Stream; import javax.annotation.Nullable; import vadl.error.Diagnostic; import vadl.error.DiagnosticList; @@ -681,21 +681,26 @@ static Node expandNode(Parser parser, Node node) { * Assembly definitions can be written with multiple identifiers to be bound to multiple * (pseudo) instructions. However, for correct further processing they need to be expanded into * multiple definitions. + * The input requires a linked list because otherwise performance would degredate. * * @param isaDefs to be expanded. * @return returns the original isaDefs with the Assemblies expanded. */ - static List expandAssemblyDefinitionsInIsa(List isaDefs) { - // FIXME: We could modify the list inplace here for performance - return isaDefs.stream() - .flatMap(def -> { - if (def instanceof AssemblyDefinition assembly) { - return new MacroExpander(Map.of(), Map.of(), def.location().fullExpandedFromStack()) - .expandAssemblies(assembly).stream(); - } - return Stream.of(def); - }) - .collect(Collectors.toCollection(ArrayList::new)); + @SuppressWarnings("NonApiType") + static LinkedList expandAssemblyDefinitionsInIsa(LinkedList isaDefs) { + for (var iter = isaDefs.listIterator(); iter.hasNext(); ) { + var def = iter.next(); + if (!(def instanceof AssemblyDefinition assembly) || assembly.identifiers.size() == 1) { + continue; + } + + var expanded = new MacroExpander(Map.of(), Map.of(), def.location().fullExpandedFromStack()) + .expandAssemblies(assembly); + iter.set(expanded.getFirst()); + expanded.subList(1, expanded.size()).forEach(iter::add); + } + + return isaDefs; } /** diff --git a/vadl/main/vadl/ast/vadl.ATG b/vadl/main/vadl/ast/vadl.ATG index 780e294fb..df5328f2c 100644 --- a/vadl/main/vadl/ast/vadl.ATG +++ b/vadl/main/vadl/ast/vadl.ATG @@ -20,6 +20,7 @@ import java.util.ArrayDeque; import java.util.Deque; import java.util.HashMap; import java.util.List; +import java.util.LinkedList; import java.util.Map; import java.util.Objects; import javax.annotation.Nullable; @@ -440,12 +441,12 @@ The #commonDefinitionList production rule lists (beside macro definitions) all c readMacroSymbols(macroTable, extendingIsa); }); .) SYM_EQ SYM_BRACE_OPEN - isaDefinitionList<.out List definitions.> (. macroTable = macroTable.pop(); .) + isaDefinitionList<.out LinkedList definitions.> (. macroTable = macroTable.pop(); .) SYM_BRACE_CLOSE (. isaDef = new InstructionSetDefinition(identifier, extending, ParserUtils.expandAssemblyDefinitionsInIsa(definitions), startLocation.join(lastTokenLoc())); macroTable.defineSymbol(isaDef); .) . - isaDefinitionList<.out List definitions.> (. definitions = new ArrayList<>(); .) + isaDefinitionList<.out LinkedList definitions.> (. definitions = new LinkedList<>(); .) = { annotations<.out List annotations.> isaDefinition (. def.withAnnotations(annotations); definitions.add(def); .) From ddce04f803d7625fdbea12dbc4c730a177d036bc Mon Sep 17 00:00:00 2001 From: flofriday Date: Fri, 15 May 2026 14:04:09 +0200 Subject: [PATCH 11/11] frontend: Fix hack of double expanding some macros Some macros were expanded twice. This was due to a bug in the macro expander where macro arguments weren't copied properly and so using it twice resulted in only two places pointing to the same node. The hack fixed that by simply expanding it once again but at great cost to performance and memory allocations. --- vadl/main/vadl/ast/AstDumper.java | 18 ++++++++++++++++++ vadl/main/vadl/ast/AstUtils.java | 21 +++++++++++++++++++++ vadl/main/vadl/ast/MacroExpander.java | 7 +++++++ vadl/main/vadl/ast/ParserUtils.java | 5 ----- 4 files changed, 46 insertions(+), 5 deletions(-) diff --git a/vadl/main/vadl/ast/AstDumper.java b/vadl/main/vadl/ast/AstDumper.java index f9592fae8..ace9d4408 100644 --- a/vadl/main/vadl/ast/AstDumper.java +++ b/vadl/main/vadl/ast/AstDumper.java @@ -42,6 +42,24 @@ public String dump(Ast ast) { return builder.toString(); } + static String debugNode(T node) { + var dumper = new AstDumper(); + node.accept(dumper); + return dumper.builder.toString(); + } + + static String debugNode(T node) { + var dumper = new AstDumper(); + node.accept(dumper); + return dumper.builder.toString(); + } + + static String debuNode(T node) { + var dumper = new AstDumper(); + node.accept(dumper); + return dumper.builder.toString(); + } + private String indentString() { var indentBy = 2; var indentCharacters = ". : ' | "; diff --git a/vadl/main/vadl/ast/AstUtils.java b/vadl/main/vadl/ast/AstUtils.java index 531641dd4..263b9f03d 100644 --- a/vadl/main/vadl/ast/AstUtils.java +++ b/vadl/main/vadl/ast/AstUtils.java @@ -18,6 +18,7 @@ import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; import java.util.stream.Collectors; import javax.annotation.Nullable; @@ -140,4 +141,24 @@ static int argumentCount(List argGroups) { return cnt; } + static boolean isFullyExpanded(Node node) { + if (node instanceof ModelDefinition + || node instanceof PlaceholderNode + || node instanceof PlaceholderDefinition + || node instanceof PlaceholderStatement + || node instanceof PlaceholderExpr + || node instanceof AsIdExpr + || node instanceof AsStrExpr) { + return false; + } + + AtomicBoolean areChildrenExpanded = new AtomicBoolean(true); + node.forEachChild(child -> { + if (!isFullyExpanded(child)) { + areChildrenExpanded.set(false); + } + }); + return areChildrenExpanded.get(); + + } } diff --git a/vadl/main/vadl/ast/MacroExpander.java b/vadl/main/vadl/ast/MacroExpander.java index c3711db18..60beb0386 100644 --- a/vadl/main/vadl/ast/MacroExpander.java +++ b/vadl/main/vadl/ast/MacroExpander.java @@ -1506,6 +1506,13 @@ private MacroMatch expandMacroMatch(MacroMatch macroMatch) { } } } + + // Need to copy the arguments becuase otherwise all usages will point to the same + // instance, but depending on their usage, they can have different names etc. + if (AstUtils.isFullyExpanded(arg)) { + arg = expandNode(arg); + } + return arg; } diff --git a/vadl/main/vadl/ast/ParserUtils.java b/vadl/main/vadl/ast/ParserUtils.java index 884588e77..081163ae1 100644 --- a/vadl/main/vadl/ast/ParserUtils.java +++ b/vadl/main/vadl/ast/ParserUtils.java @@ -669,11 +669,6 @@ static Statement castStats(Parser p, Node n) { static Node expandNode(Parser parser, Node node) { var macroExpander = new MacroExpander(Map.of(), parser.macroOverrides, List.of()); var expanded = macroExpander.expandNode(node, parser.ast); - if (parser.macroContext.isEmpty()) { - // TODO This is necessary to completely copy all nodes to not cause issues - // in symbol collection - find out why - return macroExpander.expandNode(expanded, parser.ast); - } return expanded; }