Page MenuHomePhabricator

[NVPTX] CUDA does provide malloc/free since compute capability 2.X
ClosedPublic

Authored by jdoerfert on Mar 14 2021, 10:39 AM.

Diff Detail

Event Timeline

jdoerfert created this revision.Mar 14 2021, 10:39 AM
jdoerfert requested review of this revision.Mar 14 2021, 10:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 14 2021, 10:39 AM
tra added a comment.Mar 15 2021, 10:57 AM

Tests? NVPTX used to have issues lowering libcalls at all. It would be great to verify that it works now.

In D98606#2626735, @tra wrote:

Tests? NVPTX used to have issues lowering libcalls at all. It would be great to verify that it works now.

We use malloc in the OpenMP runtime, it works, albeit slow. Where do I put a test for this?

tra added a comment.Mar 15 2021, 12:59 PM
In D98606#2626735, @tra wrote:

Tests? NVPTX used to have issues lowering libcalls at all. It would be great to verify that it works now.

We use malloc in the OpenMP runtime, it works, albeit slow. Where do I put a test for this?

test/CodeGen/NVPTX/libcall-fulfilled.ll might be a good place.

jdoerfert updated this revision to Diff 330801.Mar 15 2021, 2:00 PM

Add tests, merge D98607 into this as comment.
TBH. This patch doesn't impact the CodeGen test but only the instcombine test.

tra accepted this revision.Mar 15 2021, 2:26 PM

LGTM module couple of test nits.

llvm/test/CodeGen/NVPTX/libcall-fulfilled.ll
39–45

I'd just leave

CHECK: call.uni
CHECK: malloc

CHECK: call-uni
CHECK: free

The surrounding call setup machinery is not particularly interesting and can potentially morph due to unrelated backend changes.

llvm/test/Transforms/InstCombine/malloc_free_delete_nvptx.ll
13

I'd explicitly check that malloc/free are missing to make it obvious.

CHECK-NOT: malloc
CHECK-NOT: free
CHECK: ret void

Otherwise the test looks somewhat odd -- it's supposedly about mallof and free, but the CHECK lines don't mention either. The optimize them properly in the comment above could also be more specific about what we expect to happen.

This revision is now accepted and ready to land.Mar 15 2021, 2:26 PM
jdoerfert added inline comments.Mar 15 2021, 2:33 PM
llvm/test/Transforms/InstCombine/malloc_free_delete_nvptx.ll
13

would it be ok to just improve the comment? CHECK-NEXT does the heavy lifting here and we might want to keep the auto-generated check lines.

tra added inline comments.Mar 15 2021, 2:37 PM
llvm/test/Transforms/InstCombine/malloc_free_delete_nvptx.ll
13

That'd work too.

jdoerfert updated this revision to Diff 330822.Mar 15 2021, 3:16 PM

Addressed comments

This revision was landed with ongoing or failed builds.Mar 15 2021, 8:46 PM
This revision was automatically updated to reflect the committed changes.