-
Notifications
You must be signed in to change notification settings - Fork 98
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
[CIR] Add FuncAttrs to cir.calls #637
Conversation
cd1dc1c
to
c176445
Compare
@bcardosolopes Currently when running
This is because CallOp are created in other places outside of It makes sense to only use methods in |
c79abfe
to
5ef82f4
Compare
Thanks for your first PR, this is great!
Sounds like a good plan!
I always prefer to refactor common code and boil down to one place that actually does the final calls to |
36d7f6d
to
475cba8
Compare
I didn't see any extra helper that was calling |
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.
Overall looks good, thanks! Some extra nits before I land this.
03fdf41
to
bedeb43
Compare
2921e81
to
f6374a6
Compare
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.
Very close, one more round of review!
Looks like there are some failing tests, possibly some refactoring went wrong. You can catch those locally by running |
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.
Awesome, LGTM. If all test pass going to be landing it soon!
Some function attributes are also callsite attributes, for instance, nothrow. This means they are going to show up in both. We don't support that just yet, hence the PR. CIR has an attribute `ExtraFuncAttr` that we current use as part of `FuncOp`, see CIROps.td. This attribute also needs to be added to `CallOp` and `TryCalOp`. Right now, In `CIRGenCall.cpp: AddAttributesFromFunctionProtoType` fills in `FuncAttrs`, but doesn't use it for anything. We should use the `FuncAttrs` result to populate constructing a `ExtraFuncAttr` and add it to the aforementioned call operations.
Some function attributes are also callsite attributes, for instance, nothrow. This means they are going to show up in both. We don't support that just yet, hence the PR. CIR has an attribute `ExtraFuncAttr` that we current use as part of `FuncOp`, see CIROps.td. This attribute also needs to be added to `CallOp` and `TryCalOp`. Right now, In `CIRGenCall.cpp: AddAttributesFromFunctionProtoType` fills in `FuncAttrs`, but doesn't use it for anything. We should use the `FuncAttrs` result to populate constructing a `ExtraFuncAttr` and add it to the aforementioned call operations.
Some function attributes are also callsite attributes, for instance, nothrow. This means they are going to show up in both. We don't support that just yet, hence the PR. CIR has an attribute `ExtraFuncAttr` that we current use as part of `FuncOp`, see CIROps.td. This attribute also needs to be added to `CallOp` and `TryCalOp`. Right now, In `CIRGenCall.cpp: AddAttributesFromFunctionProtoType` fills in `FuncAttrs`, but doesn't use it for anything. We should use the `FuncAttrs` result to populate constructing a `ExtraFuncAttr` and add it to the aforementioned call operations.
Some function attributes are also callsite attributes, for instance, nothrow. This means they are going to show up in both. We don't support that just yet, hence the PR. CIR has an attribute `ExtraFuncAttr` that we current use as part of `FuncOp`, see CIROps.td. This attribute also needs to be added to `CallOp` and `TryCalOp`. Right now, In `CIRGenCall.cpp: AddAttributesFromFunctionProtoType` fills in `FuncAttrs`, but doesn't use it for anything. We should use the `FuncAttrs` result to populate constructing a `ExtraFuncAttr` and add it to the aforementioned call operations.
Some function attributes are also callsite attributes, for instance, nothrow. This means they are going to show up in both. We don't support that just yet, hence the PR. CIR has an attribute `ExtraFuncAttr` that we current use as part of `FuncOp`, see CIROps.td. This attribute also needs to be added to `CallOp` and `TryCalOp`. Right now, In `CIRGenCall.cpp: AddAttributesFromFunctionProtoType` fills in `FuncAttrs`, but doesn't use it for anything. We should use the `FuncAttrs` result to populate constructing a `ExtraFuncAttr` and add it to the aforementioned call operations.
Some function attributes are also callsite attributes, for instance, nothrow. This means they are going to show up in both. We don't support that just yet, hence the PR.
CIR has an attribute
ExtraFuncAttr
that we current use as part ofFuncOp
, see CIROps.td. This attribute also needs to be added toCallOp
andTryCalOp
.Right now, In
CIRGenCall.cpp: AddAttributesFromFunctionProtoType
fills inFuncAttrs
, but doesn't use it for anything. We should use theFuncAttrs
result to populate constructing aExtraFuncAttr
and add it to the aforementioned call operations.