Add capture of "IRUnits" as context for an MLIR Action

[mehdi_amini]

Summary

IRUnit is defined as:

using IRUnit = PointerUnion<Operation *, Region *, Block *, Value>;

The tracing::Action is extended to take an ArrayRef<IRUnit> as context to
describe an Action. It is demonstrated in the "ActionLogging" observer.

[mehdi_amini]

rebase

[Tyker]

The data-flow analysis framework has a concept very similar to IRUnit its called ProgramPoint maybe this should be shared ?

[Mogball]

mlir/include/mlir/IR/Unit.h

18

25

32

43

newline

mlir/lib/IR/AsmPrinter.cpp

681 ↗

(On Diff #500571)

where does the 2 come from?

[rriddle]

mlir/include/mlir/IR/Unit.h

26

It'd be nice not to document this in the context of actions, I'd rather have something be general.

mlir/lib/Debug/Observers/ActionLogging.cpp

10–13

Some of these could be dropped? Like Region.h?

47–48

Can you add a stream operator and simply this?

mlir/lib/IR/AsmPrinter.cpp

228 ↗

(On Diff #500571)

It'd be nice to split this change out, it feels separable. I think we could also use this here: https://github.com/llvm/llvm-project/blob/73058e330134d275070d49dba80e5fda50ec015b/mlir/lib/Tools/mlir-lsp-server/MLIRServer.cpp#L519

[mehdi_amini]

In D144814#4163506, @Tyker wrote:

The data-flow analysis framework has a concept very similar to IRUnit its called ProgramPoint maybe this should be shared ?

I'm not sure how to unify it, it feels that somehow the ProgramPoint could inherit from IRUnit but it is declared as a more extensible framework: public PointerUnion<GenericProgramPoint *, Operation *, Value, Block *> so I'm not sure how to unify.

[mehdi_amini]

address comments

mlir/include/mlir/IR/Unit.h

26

Yeah that's a remainder from a time where this was defined inside the Action.h header.

mlir/lib/IR/AsmPrinter.cpp

228 ↗

(On Diff #500571)

What about updating MLIRServer to use this new flag?

[rriddle]

mlir/include/mlir/IR/OperationSupport.h

867 ↗

(On Diff #504478)

Missing a rebase?

mlir/include/mlir/IR/Unit.h

35

Should be able to drop most of the llvm:: in this file.

mlir/lib/IR/Unit.cpp

42–46

Can you just do std::distance instead (using block->getIterator())?

55–58

Feels cleaner, but feel free to ignore

[mehdi_amini]

mlir/include/mlir/IR/OperationSupport.h

867 ↗

(On Diff #504478)

Yeah haven't updated this patch since I wrote the other one :)

[mehdi_amini]

Rebase & address comments

[mehdi_amini]

One more comment addressed

[mehdi_amini]

In D144814#4187700, @mehdi_amini wrote:

In D144814#4163506, @Tyker wrote:

The data-flow analysis framework has a concept very similar to IRUnit its called ProgramPoint maybe this should be shared ?

I'm not sure how to unify it, it feels that somehow the ProgramPoint could inherit from IRUnit but it is declared as a more extensible framework: public PointerUnion<GenericProgramPoint *, Operation *, Value, Block *> so I'm not sure how to unify.

If anyone gets an idea, we'll refactor then, in the meantime I'll push this as-is!