This is an archive of the discontinued LLVM Phabricator instance.

Fix all-reduce int tests by host-registering memrefs. Reduce amount of boiler plate to register host memory.
ClosedPublic

Authored by csigg on Mar 22 2020, 2:04 AM.

Details

Summary

Fix all-reduce int tests by host-registering memrefs.

Diff Detail

Event Timeline

csigg created this revision.Mar 22 2020, 2:04 AM
mehdi_amini added inline comments.Mar 22 2020, 10:45 AM
mlir/tools/mlir-cuda-runner/cuda-runtime-wrappers.cpp
97

FYI: clang-tidy flags this variable name

109

Why does the registration requires to overwrite the array?

csigg updated this revision to Diff 251911.EditedMar 22 2020, 12:19 PM

Fix variable name (dense_strides > denseStrides)

csigg marked 2 inline comments as done.Mar 22 2020, 12:22 PM
csigg added inline comments.
mlir/tools/mlir-cuda-runner/cuda-runtime-wrappers.cpp
97

Thanks. Fixed.

109

It's not required, and it would probably be good to remove (or have an op to allocate host-registered memory for tests). I don't want to change the functionality as part of this CL though.

herhut accepted this revision.Mar 23 2020, 3:09 AM

Thanks for fixing this. The clang-tidy error seems benign.

This revision is now accepted and ready to land.Mar 23 2020, 3:09 AM

@clementval FYI, these tests were failing.

This revision was automatically updated to reflect the committed changes.

@clementval FYI, these tests were failing.

Ok weird. There were passing on my system. Anyway thanks for fixing.