This is an archive of the discontinued LLVM Phabricator instance.

[mlir][VectorOps] Add ShapeCastOp to the vector ops dialect.
ClosedPublic

Authored by andydavis1 on Jan 29 2020, 8:45 AM.

Details

Summary

Add ShapeCastOp to the vector ops dialect.

The shape_cast operation casts between an n-D source vector shape and a k-D result vector shape (the element type remains the same).

Diff Detail

Event Timeline

andydavis1 created this revision.Jan 29 2020, 8:45 AM

Unit tests: fail. 62283 tests passed, 1 failed and 831 were skipped.

failed: libc++.std/thread/thread_mutex/thread_mutex_requirements/thread_mutex_requirements_mutex/thread_mutex_recursive/lock.pass.cpp

clang-tidy: pass.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

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

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

rriddle added inline comments.Jan 29 2020, 10:10 AM
mlir/lib/Dialect/VectorOps/VectorOps.cpp
1416

Use /// for top level comments

1476

.getOperation isn't necessary.

1492

Same here.

aartbik added inline comments.Jan 29 2020, 10:28 AM
mlir/include/mlir/Dialect/VectorOps/VectorOps.td
975

we have a mixed bag of notation (variables with and without quotes) in our descriptions in this file and at some point we should unify this; in this case, I find n > k easier to read, and the n and k do no refer back to any parameters in real code anyway

mlir/lib/Dialect/VectorOps/VectorOps.cpp
1418

technically this is not a verify since it returns a bool, not a LogicalResult.
how about hasMatchingShapeCastDims or something like that

andydavis1 marked 5 inline comments as done.Jan 30 2020, 8:49 AM

addressing comments

Unit tests: pass. 62322 tests passed, 0 failed and 838 were skipped.

clang-tidy: pass.

clang-format: pass.

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

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

addressing comments

Thanks Andy, this should make a bunch of things easier!

This revision is now accepted and ready to land.Jan 31 2020, 12:05 PM
rriddle marked an inline comment as done.Jan 31 2020, 12:14 PM
rriddle added inline comments.
mlir/lib/Dialect/VectorOps/VectorOps.cpp
1397

nit: Can you use the declarative assembly form now that it has landed? There isn't any docs yet(sorry, some stuff is still in-flight), but this one should be something like:

let assemblyFormat = "$source attr-dict type($source) to type($result)";

addressing comments

andydavis1 marked an inline comment as not done.Jan 31 2020, 2:16 PM

Addressed River's comment.

Unit tests: fail. 62357 tests passed, 1 failed and 839 were skipped.

failed: libc++.std/containers/sequences/array/array_creation/to_array.fail.cpp

clang-tidy: pass.

clang-format: pass.

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

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

Sync with github master

Unit tests: fail. 62357 tests passed, 1 failed and 839 were skipped.

failed: libc++.std/containers/sequences/array/array_creation/to_array.fail.cpp

clang-tidy: pass.

clang-format: pass.

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

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

rriddle marked an inline comment as done.Jan 31 2020, 8:57 PM
rriddle added inline comments.
mlir/lib/Dialect/VectorOps/VectorOps.cpp
1397

We should be able to remove these now, right?

rebasing with head

Unit tests: unknown.

clang-tidy: pass.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

andydavis1 marked an inline comment as done.Feb 5 2020, 1:26 PM

addressing comments

andydavis1 updated this revision to Diff 242742.Feb 5 2020, 1:36 PM

addressing comments

Unit tests: unknown.

clang-tidy: pass.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

andydavis1 updated this revision to Diff 242744.Feb 5 2020, 1:53 PM

fixed format.

Unit tests: unknown.

clang-tidy: pass.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

andydavis1 updated this revision to Diff 242762.Feb 5 2020, 3:14 PM

fixing formating take 2

andydavis1 updated this revision to Diff 242771.Feb 5 2020, 3:36 PM

rebasing with head.

Unit tests: unknown.

clang-tidy: pass.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

This revision was automatically updated to reflect the committed changes.

Unit tests: unknown.

clang-tidy: pass.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.