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
39–43
mlir/docs/Traits.md
269–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
269–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/
4 lines
8 lines
7 lines

Diff 295166

mlir/docs/Interfaces.md

Show First 20 Lines • Show All 222 Lines • ▼ Show 20 Lines
##### RegionKindInterfaces ##### RegionKindInterfaces
* `RegionKindInterface` - Used to describe the abstract semantics of regions. * `RegionKindInterface` - Used to describe the abstract semantics of regions.
- `RegionKind getRegionKind(unsigned index)` - Return the kind of the region with the given index inside this operation. - `RegionKind getRegionKind(unsigned index)` - Return the kind of the region with the given index inside this operation.
- RegionKind::Graph - represents a graph region without control flow semantics - RegionKind::Graph - represents a graph region without control flow semantics
- RegionKind::SSACFG - represents an [SSA-style control flow](LangRef.md#modeling-control-flow) region with basic blocks and reachability - RegionKind::SSACFG - represents an [SSA-style control flow](LangRef.md#modeling-control-flow) region with basic blocks and reachability
- `hasSSADominance(unsigned index)` - Return true if the region with the given index inside this operation requires dominance. - `hasSSADominance(unsigned index)` - Return true if the region with the given index inside this operation requires dominance.
##### SymbolInterfaces
* `SymbolOpInterface` - Used to represent [`Symbol`](SymbolsAndSymbolTables.md#symbol) operations which reside immediately within a region that defines a [`SymbolTable`](SymbolsAndSymbolTables.md#symbol-table).

mlir/docs/SymbolsAndSymbolTables.md

Show All 30 Lines
within the parent `SymbolTable`. This name is semantically similarly to an SSA within the parent `SymbolTable`. This name is semantically similarly to an SSA
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 should use the `SymbolOpInterface` interface to provide the
necessary verification and accessors, but this is not required as some necessary verification and accessors; it also supports
operations, such as `module`, conditionally define a symbol. `Symbol`s must have operations, such as `module`, that conditionally define a symbol. `Symbol`s must
the following properties: have 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 260 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
* `OpTrait::Symbol` -- `Symbol`
This trait is used for operations that define a
[`Symbol`](SymbolsAndSymbolTables.md#symbol).
### SymbolTable ### SymbolTable
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::SymbolTable` -- `SymbolTable` * `OpTrait::SymbolTable` -- `SymbolTable`
This trait is used for operations that define a This trait is used for operations that define a
[`SymbolTable`](SymbolsAndSymbolTables.md#symbol-table). [`SymbolTable`](SymbolsAndSymbolTables.md#symbol-table).
### Terminator ### Terminator
* `OpTrait::IsTerminator` -- `Terminator` * `OpTrait::IsTerminator` -- `Terminator`
This trait provides verification and functionality for operations that are known This trait provides verification and functionality for operations that are known
to be [terminators](LangRef.md#terminator-operations). to be [terminators](LangRef.md#terminator-operations).