Page MenuHomePhabricator

[mlir][SideEffects] Replace HasNoSideEffect with the memory effect interfaces.
ClosedPublic

Authored by rriddle on Mar 11 2020, 4:54 PM.

Details

Summary

HasNoSideEffect can now be implemented using the MemoryEffectInterface, removing the need to check multiple things for the same information. This also removes an easy foot-gun for users as 'Operation::hasNoSideEffect' would ignore operations that dynamically, or recursively, have no side effects. This also leads to an immediate improvement in some of the existing users, such as DCE, now that they have access to more information.

Depends On D76020

Diff Detail

Event Timeline

rriddle created this revision.Mar 11 2020, 4:54 PM
mehdi_amini accepted this revision.Mar 11 2020, 6:18 PM
mehdi_amini added inline comments.
mlir/lib/Dialect/LoopOps/Transforms/ParallelLoopFusion.cpp
152

I see this pattern frequently, could it be a static helper on MemoryEffectOpInterface?

bool MemoryEffectOpInterface::hasNoEffect(Operation *);
This revision is now accepted and ready to land.Mar 11 2020, 6:18 PM
rriddle updated this revision to Diff 249820.Mar 11 2020, 8:23 PM
rriddle marked 2 inline comments as done.

Resolve comments

rriddle added inline comments.Mar 11 2020, 8:47 PM
mlir/lib/Dialect/LoopOps/Transforms/ParallelLoopFusion.cpp
152

Funnily enough, I had that at the beginning but lost it somewhere. Added back.

This revision was automatically updated to reflect the committed changes.