-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Generic integer semantics #51
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I added some comments but otherwise looks good!
// CHECK: %block_id1 = "smt.int.constant"() {"value" = 0 : i64} : () -> !smt.int.int | ||
// CHECK-NEXT: %block_id2 = "smt.int.constant"() {"value" = 1 : i64} : () -> !smt.int.int | ||
// CHECK-NEXT: %block_id3 = "smt.int.constant"() {"value" = 2 : i64} : () -> !smt.int.int | ||
// CHECK: %block_id1 = "smt.int.constant"() {"value" = 0 : ui128} : () -> !smt.int.int | ||
// CHECK-NEXT: %block_id2 = "smt.int.constant"() {"value" = 1 : ui128} : () -> !smt.int.int | ||
// CHECK-NEXT: %block_id3 = "smt.int.constant"() {"value" = 2 : ui128} : () -> !smt.int.int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll fix this in another PR, but actually we have the IntAttr
attribute that can represent arbitrary bitwidth integers, it's probably better suited here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh ok nice
def are_generic_integers_defined_on( | ||
module_op: Operation, | ||
): | ||
forbidden_ops = [arith.OrI.name, arith.XOrI.name, arith.ShLI.name, arith.DivSI.name] | ||
forbidden_ops += [o.name for o in comb.Comb.operations] | ||
use_parametric_int = True | ||
for inner_op in module_op.walk(): | ||
if isinstance(inner_op, pdl.OperationOp): | ||
op_name = str(inner_op.opName).replace('"', "") | ||
if op_name in forbidden_ops: | ||
use_parametric_int = False | ||
break | ||
if isinstance(inner_op, pdl.ApplyNativeConstraintOp): | ||
constraint_name = str(inner_op.constraint_name).replace('"', "") | ||
if constraint_name == "is_minus_one": | ||
use_parametric_int = False | ||
break | ||
if isinstance(inner_op, pdl.ApplyNativeRewriteOp): | ||
constraint_name = str(inner_op.constraint_name).replace('"', "") | ||
if constraint_name == "get_width": | ||
use_parametric_int = False | ||
break | ||
return use_parametric_int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this function do? I'm not sure I understand
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I add a doc comment. Returns False if there is an operation in module_op that is not supported by generic integer semantics. Returns True otherwise. I use it later in pdl_to_smt
to decide wether I should load the generic integer semantics or the fixed one.
Generic integer semantics sound enough to be the foundation of PDL to SMT passes.