Page MenuHomePhabricator

[mlir][SideEffects] Mark the CFG only terminator operations as NoSideEffect
ClosedPublic

Authored by rriddle on Mar 9 2020, 7:01 PM.

Details

Summary

These terminator operations don't really have any side effects, and this allows for more accurate side-effect analysis for region operations. For example, currently we can't detect like a loop.for or affine.for are dead because the affine.terminator is "side effecting".

Note: Marking as NoSideEffect doesn't mean that these operations can be opaquely erased.

Depends On D75886

Diff Detail

Event Timeline

rriddle created this revision.Mar 9 2020, 7:01 PM

The plan is to get rid of NoSideEffect right? Are you just taking a temporary detour here?

The plan is to get rid of NoSideEffect right? Are you just taking a temporary detour here?

Essentially yes. I'm building up the necessary infra for replacing Operation::hasNoSideEffect with something more powerful. Setting that up will make it much easier to transition things.

herhut accepted this revision.Mar 12 2020, 4:30 AM

For the dialects I am involved with this looks reasonable.

mlir/lib/Transforms/CSE.cpp
127

This might still simplify terminator operations that we do not identify as those. Is the idea that those still have to remain side-effecting? Or will this be fixed in a future iteration with more detailed modelling?

This revision is now accepted and ready to land.Mar 12 2020, 4:30 AM
rriddle marked 2 inline comments as done.Mar 12 2020, 10:22 AM
rriddle added inline comments.
mlir/lib/Transforms/CSE.cpp
127

CSE/DCE doesn't modify the CFG(terminators) generally, so this check will remain the same regardless of effect modeling.

This revision was automatically updated to reflect the committed changes.
rriddle marked an inline comment as done.