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

[Ibis] Add ibis.this #5794

Merged
merged 2 commits into from
Aug 8, 2023
Merged

[Ibis] Add ibis.this #5794

merged 2 commits into from
Aug 8, 2023

Conversation

mortbopet
Copy link
Contributor

  • Adds an ibis.this operation to define the current scope.
  • Renames !ibis.classref to !ibis.scoperef since this type must now be able to support both classes and containers
  • Adds a ScopeOpInterface
  • ibis.get_parent_of_ref now takes an explicit type to cast the parent ref to. In most class cases, this will be an opaque type, but for containers these will reference the explicit parent type. The obvious benefit of this is that you're able to perform get_port on resolved scoperef types.

- Adds an `ibis.this` operation to define the current scope.
- Renames `!ibis.classref` to `!ibis.scoperef` since this type must now be able to support both classes and containers
- Adds a `ScopeOpInterface`
- `ibis.get_parent_of_ref` now can take an explicit type to cast the parent ref to.
Copy link
Contributor

@blakep-msft blakep-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

parser.parseArgument(thisArg, /*allowType=*/false) ||
parser.parseRParen())
return failure();
if (std::next(thisOps.begin()) != thisOps.end())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use thisOps.size() rather than comparing iterators?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getOps returns an iterator_range which has begin, end, empty. So one can either use std::distance or the above... went with the above, although std::distance is probably the better, more readable, option to infer the size.

include/circt/Dialect/Ibis/IbisInterfaces.td Outdated Show resolved Hide resolved
include/circt/Dialect/Ibis/IbisInterfaces.td Outdated Show resolved Hide resolved
include/circt/Dialect/Ibis/IbisOps.td Outdated Show resolved Hide resolved
include/circt/Dialect/Ibis/IbisTypes.td Outdated Show resolved Hide resolved
auto parentScope =
dyn_cast_or_null<ScopeOpInterface>(getOperation()->getParentOp());
if (!parentScope)
return emitOpError() << "thisOp must be nested in a scope op";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you express this in ODS with HasParent<ScopeOpInterface>?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, generated code expects a static getOperationName to be defined for the type.

@mortbopet mortbopet merged commit 99b0987 into main Aug 8, 2023
5 checks passed
@darthscsi darthscsi deleted the dev/mpetersen/ibis_this branch June 4, 2024 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants