Page MenuHomePhabricator

[MLIR][NFC] fix name operand -> op
ClosedPublic

Authored by bondhugula on Apr 4 2020, 10:11 PM.

Details

Summary

The wrong name was confusing to read. value.getUsers() yields
Operation *s.

Depends on D77483.

Diff Detail

Event Timeline

bondhugula created this revision.Apr 4 2020, 10:11 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 4 2020, 10:11 PM
stephenneuendorffer requested changes to this revision.Apr 4 2020, 10:35 PM
stephenneuendorffer added inline comments.
mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp
182–183

This seems like a bad idea to me, since you're shadowing another outer op.

This revision now requires changes to proceed.Apr 4 2020, 10:35 PM
bondhugula marked 2 inline comments as done.

Rename op -> userOp

mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp
182–183

I hadn't even noticed - thanks! Fixed.

rriddle accepted this revision.Apr 5 2020, 12:07 AM
rriddle added inline comments.
mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp
181–182

Can you change this to just op->getUsers()? It is cleaner and more efficient.

bondhugula marked 2 inline comments as done.Apr 5 2020, 4:05 AM
bondhugula added inline comments.
mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp
181–182

Yes, but we should do that in another patch - this is meant to be a rename only. There are other instances (two in this file) and about five more in other files that use such a 2-d iteration that could all be replaced.

This revision was not accepted when it landed; it landed in state Needs Review.Apr 5 2020, 6:56 AM
This revision was automatically updated to reflect the committed changes.
bondhugula marked an inline comment as done.