Skip to content
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

Const-Decl #233

Closed
acl-cqc opened this issue Jun 30, 2023 · 6 comments · Fixed by #1123
Closed

Const-Decl #233

acl-cqc opened this issue Jun 30, 2023 · 6 comments · Fixed by #1123
Assignees
Labels
breaking-change Changes that break semver

Comments

@acl-cqc
Copy link
Contributor

acl-cqc commented Jun 30, 2023

i.e. we should allow declaring that there's an external Const (in another Hugr), without giving its value here.

This may mean we need to add a "name" to each Const, paralleling {Alias,Func}{Defn,Decl}. Or, perhaps we should revisit Func{Defn,Decl} having names, and say these are metadata as they affect only the linker.

@aborgna-q aborgna-q added the breaking-change Changes that break semver label Mar 14, 2024
@ss2165
Copy link
Member

ss2165 commented Mar 14, 2024

Upping the priority of this because such named declarations would be useful for declaring circuit parameters in guppy that are later linked in at the device.

@doug-q
Copy link
Collaborator

doug-q commented Mar 18, 2024

What information does an external Const store? It must contain the type, and it would seem that it must contain a name(or at least some unique id) if a linker is to merge it with it's definition.

Linking is not the only way to eliminate an external Const though; it is also reasonable to elaborate an external Const into a "full" Const during a Hugr -> Hugr pass.

My suspicion is that when linking one would not want to constrain the "CustomConst" instance of an external const, wanting the linker to be free to merge with an arbitrary Const of the correct type. I suppose then that a Hugr -> Hugr pass that replaces some external Const nodes with full Const nodes should use either types or metadata to drive this.

@acl-cqc
Copy link
Contributor Author

acl-cqc commented Mar 18, 2024

What information does an external Const store? It must contain the type, and it would seem that it must contain a name(or at least some unique id) if a linker is to merge it with it's definition.

Agreed - much like FuncDecl (and FuncDefn) that has name: String

Linking is not the only way to eliminate an external Const though; it is also reasonable to elaborate an external Const into a "full" Const during a Hugr -> Hugr pass.

Right. So given we have the Const node that we have now, i.e. for which there is no equivalent in the world of Func(Decl/Defn), I see two possibilities:

  • An ExternalConst node with a name, that is turned into a Const at link time (copying/inlining the value from the ConstDecl)
  • An ExternalConst node with a static input, that comes from either a ConstDecl or a ConstDefn. ConstDecl has a type+name; ConstDefn has a type, presumably name, and value. Linking changes the source of the static edge - previously from the ConstDecl, becomes from the ConstDefn.

In all cases the name has no semantics outside of linking, but is probably a useful thing to display/describe.

@doug-q
Copy link
Collaborator

doug-q commented Apr 11, 2024

It is not necessary to introduce a ConstDecl. a struct ExternConst{t: Type, symbol: String} (this could be defined in prelude) can be given an impl CustomConst. When lowering to machine code this would be lowered to an external symbol. I think this is a very general approach, indeed any extension can define an analogous CustomConst with whatever linking semantics it desires.

@acl-cqc
Copy link
Contributor Author

acl-cqc commented Apr 15, 2024

It is not necessary to introduce a ConstDecl. a struct ExternConst{t: Type, symbol: String} (this could be defined in prelude) can be given an impl CustomConst. When lowering to machine code this would be lowered to an external symbol.

This would be for (a) Hugr use-cases that do not involve lowering to machine code or using a linker provided by a lower level, and/or (b) when the constant is provided by another Hugr, i.e. we are doing Hugr linking (#882) - not mutually exclusive with the previous. (Perhaps Hugr linking could be done by lowering the hugrs separately and then linking together using said lower-level linker, in Hugr use-cases which have one.)

I think this is a very general approach, indeed any extension can define an analogous CustomConst with whatever linking semantics it desires.

This is an interesting question, and if it means that we don't need a ConstDecl, then great :). (That is, if CustomConst is a powerful enough mechanism that we really don't need anything else.). If we define such a CustomConst, (a) is downcasting that to get the target symbol name/etc. easy/flexible enough to use? (b) can we metadata everything enough that these CustomConsts themselves can drive the Hugr linker? (#882 again)

@doug-q
Copy link
Collaborator

doug-q commented Apr 15, 2024

I agree that linking needs to be more general than machine-level-linking.

I think a Const containing an ExternConst is isomorphic to a hypothetical ConstDecl.

One can also imagine defining a struct ExternConstById{ t: Type, id: Option<Node>} where the id names the node in the OTHER hugr to which this Const should be linked.

One can imagine a flavour of ExternConst that points at a nexus resource.

So linking passes define the semantics of these Const nodes.

is downcasting that to get the target symbol name/etc. easy/flexible enough to use?

You will need the impl CustomConst linked in, in order to recognise the concrete type you are looking for.

can we metadata everything enough that these CustomConsts themselves can drive the Hugr linker?

I don't think so. What I have proposed is that a(!) Hugr linker would interpret some Consts as external and link them each in some specific way. One can imagine some trait (sub-traiting(?) CustomConst) to facilitate this.

@doug-q doug-q self-assigned this May 29, 2024
doug-q added a commit that referenced this issue May 29, 2024
github-merge-queue bot pushed a commit that referenced this issue May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Changes that break semver
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants