Skip to content

[CIR][CodeGen] Refactor setExtraAttributesForFunc to better align with OG #830

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

Merged
merged 1 commit into from
Sep 12, 2024

Conversation

seven-mile
Copy link
Collaborator

Previously the body of setExtraAttributesForFunc corresponds to SetLLVMFunctionAttributesForDefinition, but the callsite of it does not reside at the right position. This PR rename it and adjust the calls to it following OG CodeGen.

To be specific, setExtraAttributesForFunc is called right after the initialization of FuncOp. But in OG CodeGen, the list of attributes is constructed by several more functions: SetLLVMFunctionAttributes and SetLLVMFunctionAttributesForDefinition.

This results in diff in attributes of function declarations, which is reflected by the changes of test files. Apart from them, there is no functional change. In other words, the two code path calling setCIRFunctionAttributesForDefinition are tested by existing tests:

  • Caller buildGlobalFunctionDefinition: tested by CIR/CodeGen/function-attrs.cpp, ...
  • Caller codegenCXXStructor: tested by CIR/CodeGen/delegating-ctor.cpp, defined-pure-virtual-func.cpp, ...

bcardosolopes pushed a commit that referenced this pull request Sep 12, 2024
…`constructAttributeList` (#831)

Similar to #830 , this PR completes the `setCIRFunctionAttributes` part
with the call to `constructAttributeList` method, so that func op and
call op share the logic of handling these kinds of attributes, which is
the design of OG CodeGen.

It also includes other refactors. The function `constructAttributeList`
now use `mlir::NamedAttrList &` rather than immutable attribute
`mlir::DictionaryAttr &` as the inout result parameter, which benefits
the additive merging of attributes.
@bcardosolopes bcardosolopes merged commit bde154c into llvm:main Sep 12, 2024
7 checks passed
Hugobros3 pushed a commit to shady-gang/clangir that referenced this pull request Oct 2, 2024
…`constructAttributeList` (llvm#831)

Similar to llvm#830 , this PR completes the `setCIRFunctionAttributes` part
with the call to `constructAttributeList` method, so that func op and
call op share the logic of handling these kinds of attributes, which is
the design of OG CodeGen.

It also includes other refactors. The function `constructAttributeList`
now use `mlir::NamedAttrList &` rather than immutable attribute
`mlir::DictionaryAttr &` as the inout result parameter, which benefits
the additive merging of attributes.
Hugobros3 pushed a commit to shady-gang/clangir that referenced this pull request Oct 2, 2024
…ith OG (llvm#830)

Previously the body of `setExtraAttributesForFunc` corresponds to
`SetLLVMFunctionAttributesForDefinition`, but the callsite of it does
not reside at the right position. This PR rename it and adjust the calls
to it following OG CodeGen.

To be specific, `setExtraAttributesForFunc` is called right after the
initialization of `FuncOp`. But in OG CodeGen, the list of attributes is
constructed by several more functions: `SetLLVMFunctionAttributes` and
`SetLLVMFunctionAttributesForDefinition`.

This results in diff in attributes of function declarations, which is
reflected by the changes of test files. Apart from them, there is no
functional change. In other words, the two code path calling
`setCIRFunctionAttributesForDefinition` are tested by existing tests:

* Caller `buildGlobalFunctionDefinition`: tested by
`CIR/CodeGen/function-attrs.cpp`, ...
* Caller `codegenCXXStructor`: tested by
`CIR/CodeGen/delegating-ctor.cpp`, `defined-pure-virtual-func.cpp`, ...
smeenai pushed a commit to smeenai/clangir that referenced this pull request Oct 9, 2024
…`constructAttributeList` (llvm#831)

Similar to llvm#830 , this PR completes the `setCIRFunctionAttributes` part
with the call to `constructAttributeList` method, so that func op and
call op share the logic of handling these kinds of attributes, which is
the design of OG CodeGen.

It also includes other refactors. The function `constructAttributeList`
now use `mlir::NamedAttrList &` rather than immutable attribute
`mlir::DictionaryAttr &` as the inout result parameter, which benefits
the additive merging of attributes.
smeenai pushed a commit to smeenai/clangir that referenced this pull request Oct 9, 2024
…ith OG (llvm#830)

Previously the body of `setExtraAttributesForFunc` corresponds to
`SetLLVMFunctionAttributesForDefinition`, but the callsite of it does
not reside at the right position. This PR rename it and adjust the calls
to it following OG CodeGen.

To be specific, `setExtraAttributesForFunc` is called right after the
initialization of `FuncOp`. But in OG CodeGen, the list of attributes is
constructed by several more functions: `SetLLVMFunctionAttributes` and
`SetLLVMFunctionAttributesForDefinition`.

This results in diff in attributes of function declarations, which is
reflected by the changes of test files. Apart from them, there is no
functional change. In other words, the two code path calling
`setCIRFunctionAttributesForDefinition` are tested by existing tests:

* Caller `buildGlobalFunctionDefinition`: tested by
`CIR/CodeGen/function-attrs.cpp`, ...
* Caller `codegenCXXStructor`: tested by
`CIR/CodeGen/delegating-ctor.cpp`, `defined-pure-virtual-func.cpp`, ...
smeenai pushed a commit to smeenai/clangir that referenced this pull request Oct 9, 2024
…`constructAttributeList` (llvm#831)

Similar to llvm#830 , this PR completes the `setCIRFunctionAttributes` part
with the call to `constructAttributeList` method, so that func op and
call op share the logic of handling these kinds of attributes, which is
the design of OG CodeGen.

It also includes other refactors. The function `constructAttributeList`
now use `mlir::NamedAttrList &` rather than immutable attribute
`mlir::DictionaryAttr &` as the inout result parameter, which benefits
the additive merging of attributes.
smeenai pushed a commit to smeenai/clangir that referenced this pull request Oct 9, 2024
…ith OG (llvm#830)

Previously the body of `setExtraAttributesForFunc` corresponds to
`SetLLVMFunctionAttributesForDefinition`, but the callsite of it does
not reside at the right position. This PR rename it and adjust the calls
to it following OG CodeGen.

To be specific, `setExtraAttributesForFunc` is called right after the
initialization of `FuncOp`. But in OG CodeGen, the list of attributes is
constructed by several more functions: `SetLLVMFunctionAttributes` and
`SetLLVMFunctionAttributesForDefinition`.

This results in diff in attributes of function declarations, which is
reflected by the changes of test files. Apart from them, there is no
functional change. In other words, the two code path calling
`setCIRFunctionAttributesForDefinition` are tested by existing tests:

* Caller `buildGlobalFunctionDefinition`: tested by
`CIR/CodeGen/function-attrs.cpp`, ...
* Caller `codegenCXXStructor`: tested by
`CIR/CodeGen/delegating-ctor.cpp`, `defined-pure-virtual-func.cpp`, ...
keryell pushed a commit to keryell/clangir that referenced this pull request Oct 19, 2024
…`constructAttributeList` (llvm#831)

Similar to llvm#830 , this PR completes the `setCIRFunctionAttributes` part
with the call to `constructAttributeList` method, so that func op and
call op share the logic of handling these kinds of attributes, which is
the design of OG CodeGen.

It also includes other refactors. The function `constructAttributeList`
now use `mlir::NamedAttrList &` rather than immutable attribute
`mlir::DictionaryAttr &` as the inout result parameter, which benefits
the additive merging of attributes.
keryell pushed a commit to keryell/clangir that referenced this pull request Oct 19, 2024
…ith OG (llvm#830)

Previously the body of `setExtraAttributesForFunc` corresponds to
`SetLLVMFunctionAttributesForDefinition`, but the callsite of it does
not reside at the right position. This PR rename it and adjust the calls
to it following OG CodeGen.

To be specific, `setExtraAttributesForFunc` is called right after the
initialization of `FuncOp`. But in OG CodeGen, the list of attributes is
constructed by several more functions: `SetLLVMFunctionAttributes` and
`SetLLVMFunctionAttributesForDefinition`.

This results in diff in attributes of function declarations, which is
reflected by the changes of test files. Apart from them, there is no
functional change. In other words, the two code path calling
`setCIRFunctionAttributesForDefinition` are tested by existing tests:

* Caller `buildGlobalFunctionDefinition`: tested by
`CIR/CodeGen/function-attrs.cpp`, ...
* Caller `codegenCXXStructor`: tested by
`CIR/CodeGen/delegating-ctor.cpp`, `defined-pure-virtual-func.cpp`, ...
lanza pushed a commit that referenced this pull request Nov 5, 2024
…`constructAttributeList` (#831)

Similar to #830 , this PR completes the `setCIRFunctionAttributes` part
with the call to `constructAttributeList` method, so that func op and
call op share the logic of handling these kinds of attributes, which is
the design of OG CodeGen.

It also includes other refactors. The function `constructAttributeList`
now use `mlir::NamedAttrList &` rather than immutable attribute
`mlir::DictionaryAttr &` as the inout result parameter, which benefits
the additive merging of attributes.
lanza pushed a commit that referenced this pull request Nov 5, 2024
…ith OG (#830)

Previously the body of `setExtraAttributesForFunc` corresponds to
`SetLLVMFunctionAttributesForDefinition`, but the callsite of it does
not reside at the right position. This PR rename it and adjust the calls
to it following OG CodeGen.

To be specific, `setExtraAttributesForFunc` is called right after the
initialization of `FuncOp`. But in OG CodeGen, the list of attributes is
constructed by several more functions: `SetLLVMFunctionAttributes` and
`SetLLVMFunctionAttributesForDefinition`.

This results in diff in attributes of function declarations, which is
reflected by the changes of test files. Apart from them, there is no
functional change. In other words, the two code path calling
`setCIRFunctionAttributesForDefinition` are tested by existing tests:

* Caller `buildGlobalFunctionDefinition`: tested by
`CIR/CodeGen/function-attrs.cpp`, ...
* Caller `codegenCXXStructor`: tested by
`CIR/CodeGen/delegating-ctor.cpp`, `defined-pure-virtual-func.cpp`, ...
lanza pushed a commit that referenced this pull request Mar 18, 2025
…`constructAttributeList` (#831)

Similar to #830 , this PR completes the `setCIRFunctionAttributes` part
with the call to `constructAttributeList` method, so that func op and
call op share the logic of handling these kinds of attributes, which is
the design of OG CodeGen.

It also includes other refactors. The function `constructAttributeList`
now use `mlir::NamedAttrList &` rather than immutable attribute
`mlir::DictionaryAttr &` as the inout result parameter, which benefits
the additive merging of attributes.
lanza pushed a commit that referenced this pull request Mar 18, 2025
…ith OG (#830)

Previously the body of `setExtraAttributesForFunc` corresponds to
`SetLLVMFunctionAttributesForDefinition`, but the callsite of it does
not reside at the right position. This PR rename it and adjust the calls
to it following OG CodeGen.

To be specific, `setExtraAttributesForFunc` is called right after the
initialization of `FuncOp`. But in OG CodeGen, the list of attributes is
constructed by several more functions: `SetLLVMFunctionAttributes` and
`SetLLVMFunctionAttributesForDefinition`.

This results in diff in attributes of function declarations, which is
reflected by the changes of test files. Apart from them, there is no
functional change. In other words, the two code path calling
`setCIRFunctionAttributesForDefinition` are tested by existing tests:

* Caller `buildGlobalFunctionDefinition`: tested by
`CIR/CodeGen/function-attrs.cpp`, ...
* Caller `codegenCXXStructor`: tested by
`CIR/CodeGen/delegating-ctor.cpp`, `defined-pure-virtual-func.cpp`, ...
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.

2 participants