This is an archive of the discontinued LLVM Phabricator instance.

[CodeGen] Fix incorrect uses of getVectorNumElements()
ClosedPublic

Authored by david-arm on May 5 2020, 1:55 AM.

Details

Summary

I have fixed up some places in SelectionDAG::getNode() where we
used to assert that the number of vector elements for two types
are the same. I have changed such cases to assert that the
element counts are the same instead. I've added new tests that
exercise the code paths for all the truncations. All the extend
operations are covered by this existing test:

CodeGen/AArch64/sve-sext-zext.ll

For the ISD::SETCC case I fixed this code path is exercised by
these existing tests:

CodeGen/AArch64/sve-fcmp.ll
CodeGen/AArch64/sve-intrinsics-int-compares-with-imm.ll

Diff Detail

Event Timeline

david-arm created this revision.May 5 2020, 1:55 AM
sdesmalen added inline comments.May 5 2020, 2:36 AM
llvm/test/CodeGen/AArch64/sve-ext-trunc.ll
1

You can drop REQUIRES: asserts since this test is equally valid without asserts.

6

missing CHECK-NEXT: ret

For a patch like this, you can also generate the CHECK lines with the update_llc_test_checks.py script.

102

If this is a no-op, please add a CHECK-NEXT: ret

david-arm marked an inline comment as done.May 5 2020, 2:50 AM
david-arm added inline comments.
llvm/test/CodeGen/AArch64/sve-ext-trunc.ll
1

OK, I thought that since I was trying to test the actual code change (within an assert) that the test would only exercise that code path with asserts on. You are right though that it's equally valid without the assert, which is then just testing the codegen is correct. I assume buildbot builds and tests with asserts anyway?

david-arm updated this revision to Diff 262561.May 7 2020, 12:42 AM
david-arm edited the summary of this revision. (Show Details)
david-arm updated this revision to Diff 262562.May 7 2020, 12:47 AM
david-arm marked 3 inline comments as done.

Hi Sander, I think I've addressed your review comments now, thanks!

sdesmalen accepted this revision.May 11 2020, 1:10 PM

LGTM, cheers @david-arm

This revision is now accepted and ready to land.May 11 2020, 1:10 PM
This revision was automatically updated to reflect the committed changes.