Extend ASL support: strings, comparisons, etc. #1

alastairreid · 2025-01-10T18:42:00Z

This PR:

fixes minor errors in tests
aligns the spelling of primops with their name in ASLi
adds some new operations (e.g., comparisons)
adds some new external functions (e.g., print functions)
adds the type !asl.string and the operation asl.constant_string
adds a disgusting hack to let me use scf.if because I need help adding a proper asl.if
(this hack is in a separate commit to make it easier to revert/fix)

I have not added a test for each of the new operations yet.

If used with this branch of ASLi

https://github.com/alastairreid/asl-interpreter/tree/areid-mlir

whose HEAD is currently this commit

commit c679fe6a23d0fbedad72663a7f3311db3e83dc67 
Author: Alastair Reid <[email protected]>
Date:   Thu Jan 9 11:58:12 2025 +0000

MLIR: add lit test support

then running the following commands

export ASL_XDSL_DIR=`pwd`/../asl-xdsl
make test_backend_mlir

will run 119 tests and 17 of them will pass.

This makes it match the name used in ASLi

Also changes the spelling of some primops to match ASLi's spelling

This hack consists of - a bogus operation 'asl.bool_to_if' that fills the gap between how ASL represents bools and how scf represents them - limited implementations of scf.if and scf.yield that are limited to the case that no mutable local variables are modified in the if statement The right way to do this is to add asl.if which would 1) Support a list of elsif blocks 2) Use a region for the guard on each of the elsif blocks 3) Optionally use a region for the guard on the if block (not needed but it is more symmetric) This hack let me run a bunch of tests for operations that return boolean

superlopuh · 2025-01-10T20:54:44Z

asl_xdsl/dialects/asl.py

+@irdl_attr_definition
+class StringAttr(Data[str]):


I would recommend using the StringAttr in the builtin dialect

superlopuh · 2025-01-10T20:56:34Z

asl_xdsl/dialects/asl.py

+    @classmethod
+    def parse(cls, parser: Parser) -> ConstantStringOp:
+        """Parse the operation."""
+        value = parser.parse_str_literal()
+        attr_dict = parser.parse_optional_attr_dict()
+        return ConstantStringOp(value, attr_dict)
+
+    def print(self, printer: Printer) -> None:
+        """Print the operation."""
+        printer.print(" ", '"' + self.value.data + '"')
+        if self.attributes:
+            printer.print(" ")
+            printer.print_attr_dict(self.attributes)


Would something like this work instead?

assembly_format = "$value attr-dict"

superlopuh · 2025-01-10T20:57:14Z

asl_xdsl/dialects/asl.py

+    arg = operand_def(BoolType())
+    res = result_def(builtin.IntegerType(1))
+
+    assembly_format = "$arg `:` type($arg) `->` type($res) attr-dict"


I'm not sure why, but the attr dict tends to be before the type by convention

Suggested change

assembly_format = "$arg `:` type($arg) `->` type($res) attr-dict"

assembly_format = "$arg attr-dict `:` type($arg) `->` type($res)"

Actually, that's because I moved it to the end on the other operations.
Mostly because I hate being it at the beginning, because it is "optional" in a way.

Are there other dialects that do this? Seems to me like we should stick with the convention if not.

superlopuh · 2025-01-10T20:57:51Z

asl_xdsl/interpreters/asl.py

+        results = []  # hack that is maybe good enough for the print_bool function
+        if cond:
+            args = interpreter.run_ssacfg_region(
+                op.true_region, tuple(results), "then_region"


Suggested change

op.true_region, tuple(results), "then_region"

op.true_region, (), "then_region"

superlopuh · 2025-01-10T20:58:00Z

asl_xdsl/interpreters/asl.py

+            )
+        else:
+            args = interpreter.run_ssacfg_region(
+                op.false_region, tuple(results), "else_region"


Suggested change

op.false_region, tuple(results), "else_region"

op.false_region, (), "else_region"

superlopuh · 2025-01-10T20:58:43Z

asl_xdsl/interpreters/asl.py

@@ -40,6 +41,40 @@ def run_call(
    ) -> tuple[Any, ...]:
        return interpreter.call_op(op.callee.string_value(), args)

+    @impl(scf.IfOp)


I'm surprised this is necessary, xDSL already has an implementation for scf.If, I would have expected it to be enough

I just pushed the change that includes the xDSL version of this to main

superlopuh · 2025-01-10T21:00:28Z

tests/filecheck/dialects/asl/constant_ops.mlir

@@ -1,17 +1,20 @@
 // RUN: asl-opt %s | asl-opt %s | filecheck %s

 builtin.module {
-    %true = asl.constant_bool true {attr_dict}
-    %false = asl.constant_bool false {attr_dict}
+    %true = asl.constant_bool true {"attr_dict"}


This is my bad, I switched from a commit to just "main" in xDSL, so I have a feeling that there's drift in our versions, we recently merged a change that avoide the quotes when their content is a valid symbol, so the version on the left in this diff is actually the more up to date spelling.

math-fehr · 2025-01-11T01:13:05Z

Very nice!

alastairreid added 8 commits January 9, 2025 14:53

Fix tests: mostly punctuation and quoting

9a0cc79

Change to asl.negate_int to asl.neg_int

c280e29

This makes it match the name used in ASLi

Allow negative int constants

fbd25c3

fix name of function in test

acfa33e

tests: fix names of operations

8d6ab37

ASL: add more types and operations

ffab6a3

Also changes the spelling of some primops to match ASLi's spelling

Tests: new tests for string type

cb451ed

alastairreid requested review from superlopuh and math-fehr January 10, 2025 18:42

superlopuh reviewed Jan 10, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend ASL support: strings, comparisons, etc. #1

Extend ASL support: strings, comparisons, etc. #1

alastairreid commented Jan 10, 2025

superlopuh Jan 10, 2025

superlopuh Jan 10, 2025 •

edited

Loading

superlopuh Jan 10, 2025

math-fehr Jan 11, 2025

superlopuh Jan 11, 2025

superlopuh Jan 10, 2025

superlopuh Jan 10, 2025

superlopuh Jan 10, 2025

superlopuh Jan 10, 2025

superlopuh Jan 10, 2025

math-fehr commented Jan 11, 2025

	assembly_format = "$arg `:` type($arg) `->` type($res) attr-dict"
	assembly_format = "$arg attr-dict `:` type($arg) `->` type($res)"

	op.true_region, tuple(results), "then_region"
	op.true_region, (), "then_region"

	op.false_region, tuple(results), "else_region"
	op.false_region, (), "else_region"

		@irdl_attr_definition
		class StringAttr(Data[str]):

Extend ASL support: strings, comparisons, etc. #1

Are you sure you want to change the base?

Extend ASL support: strings, comparisons, etc. #1

Conversation

alastairreid commented Jan 10, 2025

Choose a reason for hiding this comment

superlopuh Jan 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

math-fehr commented Jan 11, 2025

superlopuh Jan 10, 2025 •

edited

Loading