This is an archive of the discontinued LLVM Phabricator instance.

[StreamExecutor] Read dev array directly in test
ClosedPublic

Authored by jhen on Sep 1 2016, 12:04 PM.

Details

Summary

Step 2 of getting GlobalDeviceMemory to own its handle.

Use the SimpleHostPlatformDevice allocate methods to create device
arrays for tests, and check for successful copies by dereferncing the
device array handle directly because we know it is really a host
pointer.

Diff Detail

Repository
rL LLVM

Event Timeline

jhen updated this revision to Diff 70044.Sep 1 2016, 12:04 PM
jhen retitled this revision from to [StreamExecutor] Read dev array directly in test.
jhen updated this object.
jhen added a reviewer: jlebar.
jhen added subscribers: parallel_libs-commits, jprice.
jlebar added inline comments.Sep 1 2016, 12:10 PM
streamexecutor/lib/unittests/SimpleHostPlatformDevice.h
30 ↗(On Diff #70044)

Hm, I just noticed this whole file is in the global namespace. Is that intentional?

138 ↗(On Diff #70044)

This seems like an awfully generic name... I guess it's only for unit tests, but still...

It could be a static function on SimpleHostPlatformDevice?

jhen marked an inline comment as done.Sep 1 2016, 2:29 PM
jhen added inline comments.
streamexecutor/lib/unittests/SimpleHostPlatformDevice.h
30 ↗(On Diff #70044)

I thought that was the right thing to do for test code, but looking at LLVM and clang shows that's not how it's done there. They put header test code in namespace llvm and clang, respectively. Still, I don't want this code on an equal footing with the library code, so I put it in namespace streamexecutor::test. Does that seem reasonable?

138 ↗(On Diff #70044)

You're right, I think it works better as a static function in SimpleHostPlatformDevice. I moved it there.

jhen updated this revision to Diff 70071.Sep 1 2016, 2:29 PM
jhen marked an inline comment as done.
  • Respond to jlebar's comments
jlebar accepted this revision.Sep 1 2016, 4:31 PM
jlebar edited edge metadata.
jlebar added inline comments.
streamexecutor/lib/unittests/SimpleHostPlatformDevice.h
33–34 ↗(On Diff #70071)

sgtm

This revision is now accepted and ready to land.Sep 1 2016, 4:31 PM
This revision was automatically updated to reflect the committed changes.