This is an archive of the discontinued LLVM Phabricator instance.

[NVPTX][tests] Do not run the tests which are not supported by nvptx
ClosedPublic

Authored by i-chebykin on Apr 18 2022, 5:07 AM.

Details

Summary

Some generic tests are not supported by the nvptx now.
Moreover, they are no plans to fix the tested features in nvptx.
So, suggest to mark them as UNSUPPORTED

Diff Detail

Event Timeline

i-chebykin created this revision.Apr 18 2022, 5:07 AM
i-chebykin requested review of this revision.Apr 18 2022, 5:07 AM
tra added inline comments.Apr 18 2022, 11:14 AM
llvm/test/CodeGen/Generic/2007-12-17-InvokeAsm.ll
4

This should probably be "REQUIRE: x86" instead, as it uses inline assembly with x86-specific constraints.

llvm/test/CodeGen/Generic/2010-ZeroSizedArg.ll
4

What exactly is the problem with this case -- the datatype with 0-element fields or vararg function printf?
If that's the latter, I think printf could be removed as it's unused by the test.

llvm/test/CodeGen/Generic/APIntLoadStore.ll
3

This kind of tests should probably be cloned into NVPTX-specific variants with appropriate REQUIRES tag. We do want to make sure that we do not regress on the types NVPTX does support.

llvm/test/CodeGen/Generic/empty-load-store.ll
4

What happens in this case? AFAICT, llc appears to work with this IR: https://godbolt.org/z/n1eK193br

llvm/test/CodeGen/Generic/pr12507.ll
8

This test crashes LLVM, probably due to the use of i160. https://godbolt.org/z/W6s4n4vM7
We need at least a comment about that.

llvm/test/CodeGen/Generic/pr24662.ll
4

Another crasher: https://godbolt.org/z/cYPebKasG, probably due to i670010. Please add a comment.

llvm/test/CodeGen/Generic/stacksave-restore.ll
3

Crasher: https://godbolt.org/z/67583fq9q

This time due to use of @llvm.stacksave and @llvm.stackrestore.

llvm/test/CodeGen/Generic/zero-sized-array.ll
4

This one compiles to PTX: https://godbolt.org/z/c3sE14j46
What's the reason for disabling it? Something to do with [0 x i8] ?

I suspect ptxas would be unhappy about .param .align 1 .b8 f4_param_0[0].

llvm/test/CodeGen/MLRegalloc/default-eviction-advisor.ll
13–17

Again, it would be good to explain what's going on here. Considering that we don't see the message regardless of what -regalloc-enable-advisor is set to, I guess it's just not enalbed for NVPTX.

llvm/test/Feature/optnone-llc.ll
9

This test makes implicit assumptions about machine-specific pass pipeline. It might be better to either trim the list of checks to the passes that are universal for all back-ends or restrict the test only to the back-end which does match these assumptions.

Excluding NVPTX is OK, but my guess is that this test may currently be failing on other 'exotic' back-ends.

llvm/test/MC/AsmParser/include.ll
2

My guess is that the test is not universally applicable, as not all back-ends may support .include or .macro even if they do have support for parsing assembly.
OK to mark NVPTX unsupported, but the test may be problematic for other back-ends, too.

llvm/test/MC/AsmParser/macro-same-context.ll
3

Ditto.

Updated the patch

llvm/test/CodeGen/Generic/2007-12-17-InvokeAsm.ll
4

REQUIRES is matched against features only,
so we need to use REQUIRES: x86{{.*}} or maybe REQUIRES: {{.*}}x86{{.*}}
Is it good?

llvm/test/CodeGen/Generic/2010-ZeroSizedArg.ll
4

NVPTX does not support empty type

llvm/test/CodeGen/Generic/empty-load-store.ll
4

llc from assertions trunk crashed
my build with -DLLVM_ENABLE_ASSERTIONS=On crashed too

llvm/test/CodeGen/Generic/zero-sized-array.ll
4

Again, there is a crash if assets are enabled:
empty aggregate type not expected

llvm/test/CodeGen/MLRegalloc/default-eviction-advisor.ll
13–17

Right,
regalloc-enable-advisor is not used for NVPTX

i-chebykin removed rG LLVM Github Monorepo as the repository for this revision.

The patch was updated according to the review

The Generic/APIntLoadStore.ll was duplicated to NVPTX/APIntLoadStore.ll
Also, the new NVPTX/APIntLoadStore.ll was edited for nvptx-acceptable iNN

tra added a comment.Apr 19 2022, 1:58 PM

Few more nits.
LGTM otherwise.

llvm/test/CodeGen/Generic/2007-12-17-InvokeAsm.ll
4

No idea. Considering that nobody else complains about it, it's OK to just disable it for nvptx for now.

llvm/test/CodeGen/Generic/APIntParam.ll
3

"; NVPTX does not support arbitrary integer types and has acceptable subset tested in NVPTX/APIntParam.ll"

llvm/test/CodeGen/Generic/APIntSextParam.ll
3

ditto.

llvm/test/CodeGen/Generic/APIntZextParam.ll
3

ditto.

llvm/test/CodeGen/Generic/empty-load-store.ll
4

Please add a note here and elsewhere. "Triggers a crash on assertion as NVPTX does not support 0-sized arrays.".
This particular case may actually be a bug.

Updated comments and new files:
NVPTX/APIntLoadStore.ll
NVPTX/APIntParam.ll
NVPTX/APIntSextParam.ll
NVPTX/APIntZextParam.ll

tra accepted this revision.Apr 20 2022, 12:29 PM
This revision is now accepted and ready to land.Apr 20 2022, 12:29 PM

I do not have commit access yet. Could someone help me to land the patch?

This revision was landed with ongoing or failed builds.Apr 26 2022, 7:27 AM
This revision was automatically updated to reflect the committed changes.