This is an archive of the discontinued LLVM Phabricator instance.

Differential D88512

[mlir] Update docs referencing OpTrait::Symbol.
ClosedPublic

Authored by scotttodd on Sep 29 2020, 1:48 PM.

Details

Reviewers
rriddle
Commits
rG1c5aa8aeca29: [mlir] Update docs referencing OpTrait::Symbol.
Summary

Since https://reviews.llvm.org/D78522, Symbol is not a Trait itself.

Diff Detail

Event Timeline

scotttodd created this revision.Sep 29 2020, 1:48 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 29 2020, 1:48 PM
Herald added subscribers: tatianashp, msifontes, jurahul and 13 others. · View Herald Transcript
scotttodd requested review of this revision.Sep 29 2020, 1:48 PM
Herald added subscribers: stephenneuendorffer, nicolasvasilache. · View Herald TranscriptSep 29 2020, 1:48 PM
scotttodd added a reviewer: rriddle.Sep 29 2020, 1:49 PM
Harbormaster completed remote builds in B73419: Diff 295106.Sep 29 2020, 1:53 PM
rriddle accepted this revision.Sep 29 2020, 2:19 PM

Thanks for fixing this!

mlir/docs/SymbolsAndSymbolTables.md
40–43
mlir/docs/Traits.md
270

Can you move this to Interfaces.md now instead? It isn't really a trait anymore.

This revision is now accepted and ready to land.Sep 29 2020, 2:19 PM
scotttodd updated this revision to Diff 295123.Sep 29 2020, 2:36 PM

Move Symbol docs to Traits.md and update language.

Harbormaster completed remote builds in B73425: Diff 295123.Sep 29 2020, 2:37 PM
scotttodd marked an inline comment as done.Sep 29 2020, 2:38 PM
scotttodd added inline comments.
mlir/docs/Traits.md
270

Moved it and tried to match the documentation style in that file with the new section.

scotttodd added a comment.Sep 29 2020, 3:58 PM

BTW, this is my first patch to LLVM/MLIR, so I think I'll need help submitting/merging. Not sure about the "patch does not apply" build failures - I think there are no conflicts...

mehdi_amini added a comment.Sep 29 2020, 8:40 PM

Arc says " INFO Base commit is not in local repository; trying to fetch." ; seems like you have local changes?

The patch also does not apply on top of master for me.

mehdi_amini added a comment.Sep 29 2020, 8:41 PM

(you local commit is based of f20918b4b15 apparently)

scotttodd updated this revision to Diff 295166.Sep 29 2020, 8:46 PM

Rebase on master.

Harbormaster completed remote builds in B73452: Diff 295166.Sep 29 2020, 8:50 PM
scotttodd added a comment.Sep 29 2020, 8:51 PM
In D88512#2302324, @mehdi_amini wrote:

(you local commit is based of f20918b4b15 apparently)

I had stacked a second git commit on top of my original commit and ran arc diff --update D88512. I'm following https://secure.phabricator.com/book/phabricator/article/arcanist_diff/ but coming from a git background I don't quite understand the relationship between local commits and what shows up here. I just rebased onto tip of tree, squashed my commits, and updated again. Might be better now?

Closed by commit rG1c5aa8aeca29: [mlir] Update docs referencing OpTrait::Symbol. (authored by scotttodd, committed by mehdi_amini). · Explain WhySep 29 2020, 8:56 PM
This revision was automatically updated to reflect the committed changes.
mehdi_amini added a commit: rG1c5aa8aeca29: [mlir] Update docs referencing OpTrait::Symbol..

Revision Contents

PathSize
mlir/
docs/
2 lines
2 lines

Diff 295106

mlir/docs/SymbolsAndSymbolTables.md

Show All 31 Lines
result value, and may be referred to by other operations to provide a symbolic result value, and may be referred to by other operations to provide a symbolic
link, or use, to the symbol. An example of a `Symbol` operation is link, or use, to the symbol. An example of a `Symbol` operation is
[`func`](LangRef.md#functions). `func` defines a symbol name, which is [`func`](LangRef.md#functions). `func` defines a symbol name, which is
[referred to](#referencing-a-symbol) by operations like [referred to](#referencing-a-symbol) by operations like
[`std.call`](Dialects/Standard.md#call). [`std.call`](Dialects/Standard.md#call).
### Defining a Symbol ### Defining a Symbol
A `Symbol` operation may use the `OpTrait::Symbol` trait to provide the A `Symbol` operation may use the `SymbolOpInterface::Trait` trait to provide the
necessary verification and accessors, but this is not required as some necessary verification and accessors, but this is not required as some
operations, such as `module`, conditionally define a symbol. `Symbol`s must have operations, such as `module`, conditionally define a symbol. `Symbol`s must have
the following properties: the following properties:
rriddleUnsubmitted
Done
ReplyInline Actions
### Defining a Symbol
- A `Symbol` operation may use the `SymbolOpInterface::Trait` trait to provide the
- necessary verification and accessors, but this is not required as some
- operations, such as `module`, conditionally define a symbol. `Symbol`s must have
+ A `Symbol` operation should use the `SymbolOpInterface` interface to provide the
+ necessary verification and accessors; it also supports
+ operations, such as `module`, that conditionally define a symbol. `Symbol`s must have
the following properties:
* A `StringAttr` attribute named
rriddle:
* A `StringAttr` attribute named * A `StringAttr` attribute named
'SymbolTable::getSymbolAttrName()'(`sym_name`). 'SymbolTable::getSymbolAttrName()'(`sym_name`).
- This attribute defines the symbolic 'name' of the operation. - This attribute defines the symbolic 'name' of the operation.
* An optional `StringAttr` attribute named * An optional `StringAttr` attribute named
'SymbolTable::getVisibilityAttrName()'(`sym_visibility`) 'SymbolTable::getVisibilityAttrName()'(`sym_visibility`)
- This attribute defines the [visibility](#symbol-visibility) of the - This attribute defines the [visibility](#symbol-visibility) of the
symbol, or more specifically in-which scopes it may be accessed. symbol, or more specifically in-which scopes it may be accessed.
▲ Show 20 Lines • Show All 165 Lines • Show Last 20 Lines

mlir/docs/Traits.md

Show First 20 Lines • Show All 261 Lines • ▼ Show 20 Lines
### Single Block with Implicit Terminator ### Single Block with Implicit Terminator
* `OpTrait::SingleBlockImplicitTerminator<typename TerminatorOpType>` : * `OpTrait::SingleBlockImplicitTerminator<typename TerminatorOpType>` :
`SingleBlockImplicitTerminator<string op>` `SingleBlockImplicitTerminator<string op>`
This trait provides APIs and verifiers for operations with regions that have a This trait provides APIs and verifiers for operations with regions that have a
single block that must terminate with `TerminatorOpType`. single block that must terminate with `TerminatorOpType`.
### Symbol ### Symbol
rriddleUnsubmitted
Not Done
ReplyInline Actions

Can you move this to Interfaces.md now instead? It isn't really a trait anymore.

rriddle: Can you move this to Interfaces.md now instead? It isn't really a trait anymore.
scotttoddAuthorUnsubmitted
Done
ReplyInline Actions

Moved it and tried to match the documentation style in that file with the new section.

scotttodd: Moved it and tried to match the documentation style in that file with the new section.
* `OpTrait::Symbol` -- `Symbol` * `SymbolOpInterface::Trait` -- `Symbol`
This trait is used for operations that define a This trait is used for operations that define a
[`Symbol`](SymbolsAndSymbolTables.md#symbol). [`Symbol`](SymbolsAndSymbolTables.md#symbol).
### SymbolTable ### SymbolTable
* `OpTrait::SymbolTable` -- `SymbolTable` * `OpTrait::SymbolTable` -- `SymbolTable`
Show All 9 Lines