This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Update the use-list algorithms in SymbolTable to support nested references.
ClosedPublic

Authored by rriddle on Dec 31 2019, 3:45 PM.

Details

Summary

This updates the use list algorithms to support querying from a specific symbol, allowing for the collection and detection of nested references. This works by walking the parent "symbol scopes" and applying the existing algorithm at each level.

Diff Detail

Event Timeline

rriddle created this revision.Dec 31 2019, 3:45 PM

Unit tests: pass. 61127 tests passed, 0 failed and 728 were skipped.

clang-tidy: fail. Please fix clang-tidy findings.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

jpienaar accepted this revision.Jan 6 2020, 12:57 PM

I don't know this part as well as Mehdi does, but it looks good.

mlir/lib/IR/SymbolTable.cpp
29

So we'd have nullptr returned if either a op has no parent or if its parent is a unknown op? This seems sort of difficult to interpret for a caller (e.g., I found no symbol table or I found an unregistered op). Or what should be done when nullptr is returned? [and could you add that to the comment describing the functions]

Also do we have a test that exercises this?

201

So we don't need to assert here as to the type as we are effectively invoking line 188 here? I'd be pro asserting here too just as it makes the precondition clear.

396

missing space ?

This revision is now accepted and ready to land.Jan 6 2020, 12:57 PM
rriddle updated this revision to Diff 237203.Jan 9 2020, 2:37 PM
rriddle marked 4 inline comments as done.

Address comments.

rriddle added inline comments.Jan 9 2020, 2:40 PM
mlir/lib/IR/SymbolTable.cpp
29

Error handling is generally left up to the caller, to either assert or emit an error. Added a comment about the failure case.

Unit tests: pass. 61656 tests passed, 0 failed and 778 were skipped.

clang-tidy: fail. Please fix clang-tidy findings.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

This revision was automatically updated to reflect the committed changes.