Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BugFix] fix nvptx not supported by device_enabled error #9585

Merged
merged 2 commits into from
Nov 25, 2021

Conversation

ZQPei
Copy link
Contributor

@ZQPei ZQPei commented Nov 25, 2021

Fix Issue #9513.

I think this bug was mistakenly introduced by #8032.

Copy link
Member

@junrushao junrushao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@junrushao junrushao linked an issue Nov 25, 2021 that may be closed by this pull request
@ZQPei ZQPei requested a review from kparzysz-quic as a code owner November 25, 2021 08:02
@ZQPei
Copy link
Contributor Author

ZQPei commented Nov 25, 2021

@junrushao1994
I think thir pr needs a further review, because another error occurs in ci https://ci.tlcpack.ai/blue/organizations/jenkins/tvm/detail/PR-9585/1/pipeline/267.

Quoted error is

E File "/workspace/src/target/llvm/codegen_nvptx.cc", line 146
E TVMError: Do not support sync shared.dyn

This error is raised because of shared.dyn not supported by "nvptx" target.
And it was previously covered by device_enabled("nvptx") returning False in the last few months.

@tvm.testing.requires_gpu
def test_dyn_shared():
n = te.size_var("n")
dtype = "float32"
A = te.placeholder((n,), name="A")
def test_device_ir(A, B):
n = A.shape[0]
ib = tvm.tir.ir_builder.create()
tx = te.thread_axis("threadIdx.x")
ib.scope_attr(tx, "thread_extent", n)
temp = ib.allocate(dtype, (n,), scope="shared.dyn") # n is symbolic size
Aptr = ib.buffer_ptr(A)
Bptr = ib.buffer_ptr(B)
temp[tx] = Aptr[tx]
depth = tvm.tir.log2(cast(n, "float32"))
with ib.for_range(0, depth) as i:
ib.emit(tvm.tir.Call(None, "tir.tvm_storage_sync", tvm.runtime.convert(["shared"])))
d = n >> (i + 1)
with ib.if_scope(tx < d):
temp[tx] += temp[tx + d]
Bptr[0] = temp[0]
return ib.get()
B = te.extern(
(1,),
[A],
lambda ins, outs: test_device_ir(ins[0], outs[0]),
name="reduce",
dtype=dtype,
)
s = te.create_schedule(B.op)
def check_target(target):
if not tvm.testing.device_enabled(target):
return
freduce = tvm.build(s, [A, B], target)
dev = tvm.device(target, 0)
for n in [512, 1024]:
a = tvm.nd.array(np.random.uniform(size=n).astype(A.dtype), dev)
b = tvm.nd.array(np.zeros(1, dtype=B.dtype), dev)
freduce(a, b)
tvm.testing.assert_allclose(b.numpy()[0], np.sum(a.numpy()), 1e-4, 1e-4)
for target in ["cuda", "nvptx"]:
check_target(target)

I have tried to fix it with this commit a8d14b1, and it works fine on my 2080ti gpu card.
But I am not sure if it is really OK.

Copy link
Member

@vinx13 vinx13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The codegen fix looks good

@junrushao
Copy link
Member

Yeah I think it's reasonable to handle dynamic shared memory this way for now

@junrushao junrushao merged commit fb4b7e2 into apache:main Nov 25, 2021
@junrushao
Copy link
Member

Thank you so much for the fix! @ZQPei

dchauhan-arm pushed a commit to dchauhan-arm/tvm that referenced this pull request Nov 29, 2021
* [BugFix] fix nvptx not supported by device_enabled error

Signed-off-by: ZQPei <[email protected]>

* [BugFix] shared.dyn support for codegen_nvptx

Signed-off-by: ZQPei <[email protected]>
mehrdadh pushed a commit to mehrdadh/tvm that referenced this pull request Dec 1, 2021
* [BugFix] fix nvptx not supported by device_enabled error

Signed-off-by: ZQPei <[email protected]>

* [BugFix] shared.dyn support for codegen_nvptx

Signed-off-by: ZQPei <[email protected]>
mehrdadh pushed a commit to mehrdadh/tvm that referenced this pull request Dec 1, 2021
* [BugFix] fix nvptx not supported by device_enabled error

Signed-off-by: ZQPei <[email protected]>

* [BugFix] shared.dyn support for codegen_nvptx

Signed-off-by: ZQPei <[email protected]>
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 7, 2022
* [BugFix] fix nvptx not supported by device_enabled error

Signed-off-by: ZQPei <[email protected]>

* [BugFix] shared.dyn support for codegen_nvptx

Signed-off-by: ZQPei <[email protected]>
yangulei pushed a commit to yangulei/tvm that referenced this pull request Jan 11, 2022
* [BugFix] fix nvptx not supported by device_enabled error

Signed-off-by: ZQPei <[email protected]>

* [BugFix] shared.dyn support for codegen_nvptx

Signed-off-by: ZQPei <[email protected]>
yangulei pushed a commit to yangulei/tvm that referenced this pull request Jan 12, 2022
* [BugFix] fix nvptx not supported by device_enabled error

Signed-off-by: ZQPei <[email protected]>

* [BugFix] shared.dyn support for codegen_nvptx

Signed-off-by: ZQPei <[email protected]>
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
* [BugFix] fix nvptx not supported by device_enabled error

Signed-off-by: ZQPei <[email protected]>

* [BugFix] shared.dyn support for codegen_nvptx

Signed-off-by: ZQPei <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] NVPTX as a testable device seems to be not supported anymore
3 participants