[PATCH] D80358: [MLIR] Add OpTrait::DominanceFreeScope

Mehdi AMINI via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 21 19:30:16 PDT 2020


mehdi_amini requested changes to this revision.
mehdi_amini added a comment.
This revision now requires changes to proceed.

FYI, River is OOO and back next Wednesday, and I'd *really* want to have him sign-off on this.

Concretely on this patch, while the implementation is not very intrusive, there are a few things I'd like to see discussed/addressed/planned:

- the documentation should be carefully updated: the code says " For more details, see `Traits.md#DominanceFreeScope`" but Traits.md isn't there?
- what is the status of the passes in MLIR? What would break when there is a DominanceFreeScope? Are we gonna update the infra to check for this property everywhere? The conversion framework expects operands to be visited before we convert an op for example.
- There is the syntactic/structural property of dominance that you relax here, piggybacking on the "unreachable blocks" relaxed semantics. However the definition of a basic block is that operation are executed in sequence: I am not sure what are the implication here about this?

(I'll post this in the thread as well, maybe it is better to discuss there)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80358/new/

https://reviews.llvm.org/D80358





More information about the llvm-commits mailing list