frontend: Group Array Type by rascmatt · Pull Request #976 · OpenVADL/openvadl

rascmatt · 2026-05-22T15:01:54Z

The group definition's name can be used in expressions (within group annotations) to access the matched instructions / operations at runtime. In addition it exposes a length and bitLength field.

VLIW.length // number of instructions in the group
VLIW.bitLength // number of bits in the group

VLIW(1) // the second instruction in the group
VLIW(VLIW.length - 1) // the last instruction in the group

I introduced a more general 'ArrayType' for such group references. In this case we're able to upper-bound the length and bitLength, so we can also give these properties a UIntType type with some maximal width.

Jozott00 · 2026-05-24T17:05:58Z

I'll wait with the review until #918 got merged, so it becomes easier to see this PR's changes.

Jozott00

Since this is unrelated to VIAM or ISS, I’ll leave the approval to @flofriday.

rascmatt · 2026-05-25T08:28:45Z

It will need to be added to the VIAM as well, but I didn't get to that yet. I switched it to a Draft PR, and amend it later.

flofriday

Looks pretty good.
I obviously don't get why there is a need for the ArrayType 😅
Or how the groups work and I also couldn't find a lot in our documentation.

Maybe you can explain it in a comment and then disregard all the other comments.

flofriday · 2026-05-25T17:45:00Z

+//  error: Invalid annotation expression
+//       ╭── test/resources/frontend-snapshots/typechecker/invalidGroupReference.vadl:27:13
+//       │
+//    27 │   [assert : VLIW.a = 0b0]
+//       │             ^^^^^^^^^^^^ Unable to determine type
+//       │
+//       help: Check additional errors which may have caused the type to be missing


nitpick: I don't like this error, I think we can completely remove it as it doesn't add any useful information.

flofriday · 2026-05-25T17:47:02Z

+//    27 │   [assert : VLIW.a = 0b0]
+//       │             ^^^^^^
+//       │
+//       Arrays only have the fields `length` or `bitLength`


question: Will this also be called an "Array" in the docs? Or is it something more specific like a Group Array?

I'm mostly asking because it could be confusing for users to name them Array when they only exist in a subsection of the language when the more useful analog from other languages is a Tensor.

flofriday · 2026-05-25T17:48:04Z

+//       Intersection format `(AType ∩ BType)` doesn't have any field with this name
+//       help: Did you maybe mean one of: a, b


**praise This is very readable, and a great error message. 🎉

flofriday · 2026-05-25T17:49:53Z

+//    27 │   [assert : VLIW(1)(2).a = 0b0]
+//       │             ^^^^^^^
+//       │
+//       Type `a: Bits<20>, b: Bits<5>` cannot be indexed or sliced


minor: I don't love the Type description, maybe GroupType<a: Bits<20>, b: Bits<5>> makes it clearer?
I think we have something similar for FormatTypes.

flofriday · 2026-05-25T17:53:10Z

+  public static ArrayType array(Type elementType, UIntType lengthType, UIntType bitLengthType) {
+    var hash = Objects.hash(elementType, lengthType, bitLengthType);
+    return arrayTypes
+        .computeIfAbsent(hash, k -> new ArrayType(elementType, lengthType, bitLengthType));


issue: This has hash-collisions. You need to wrap the contents in a datastructure, like a Triple.

flofriday · 2026-05-25T18:01:27Z

+
+    private final ConstantEvaluator constantEvaluator;
+
+    public GroupExprLengthCollector(ConstantEvaluator constantEvaluator) {


nitpick: Maybe this too should be a private constructor.

flofriday · 2026-05-25T18:02:06Z

+
+    private final ConstantEvaluator constantEvaluator;
+
+    public GroupExprBitLengthCollector(ConstantEvaluator constantEvaluator) {


nitpick: Same private constructor here.

flofriday · 2026-05-25T18:06:48Z

-    }
-
    Type currType = typeBeforeSlice;
+    SourceLocation targetLoc = expr.location();


nitpick: Calculating expression can be expensive in hot loops so let's add it to the only branch where we actually need it when there is an issue we need to report.

In general we optimize for the case where no errors occur.

flofriday · 2026-05-25T18:07:26Z

+
+      if (!(currType instanceof BitsType) && !(currType instanceof TensorType)
+          && !(currType instanceof ArrayType)) {
+        var loc = expr.target.location().join(targetLoc.location());


minor: The .location() call is redundant on targetLoc.location() as it's already a SourceLocation.

flofriday · 2026-05-25T18:10:01Z

+        };
      } else {
-        addErrorAndStopChecking(error("Cannot resolve `%s`".formatted(fieldName), expr)
+        throw addErrorAndStopChecking(error("Cannot resolve `%s`".formatted(fieldName), expr)


question: Is there a reason you added the throw?

rascmatt self-assigned this May 22, 2026

github-actions Bot added enhancement New feature or request lcb This is LCB related frontend This is frontend related iss This is ISS related labels May 22, 2026

rascmatt force-pushed the feature/group-array-type branch 3 times, most recently from b57ba75 to 1e86643 Compare May 24, 2026 07:45

rascmatt marked this pull request as ready for review May 24, 2026 07:54

rascmatt requested review from Jozott00 and flofriday May 24, 2026 07:54

rascmatt added 3 commits May 25, 2026 09:09

frontend: Implement group array expression type

e046b6a

Add bitLength property

3122749

Add index out of bounds check

19f1b2b

rascmatt force-pushed the feature/group-array-type branch from 1e86643 to 19f1b2b Compare May 25, 2026 07:28

Jozott00 reviewed May 25, 2026

View reviewed changes

rascmatt marked this pull request as draft May 25, 2026 08:28

flofriday reviewed May 25, 2026

View reviewed changes

		// Intersection format `(AType ∩ BType)` doesn't have any field with this name
		// help: Did you maybe mean one of: a, b


		private final ConstantEvaluator constantEvaluator;

		public GroupExprLengthCollector(ConstantEvaluator constantEvaluator) {


		private final ConstantEvaluator constantEvaluator;

		public GroupExprBitLengthCollector(ConstantEvaluator constantEvaluator) {

Conversation

rascmatt commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Jozott00 commented May 24, 2026

Uh oh!

Jozott00 left a comment

Choose a reason for hiding this comment

Uh oh!

rascmatt commented May 25, 2026

Uh oh!

flofriday left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rascmatt commented May 22, 2026 •

edited

Loading