This is an archive of the discontinued LLVM Phabricator instance.

[NFC][AArch64] Precommit test to optimize instruction selection for aarch64_neon_pmull64 intrinsic.
ClosedPublic

Authored by mingmingl on Aug 2 2022, 11:11 PM.

Details

Summary

Add test cases when instruction selection for aarch64_neon_pmull64 intrinsic is sub-optimal.

This is to prepare for D131047, and makes diff clearer.

Diff Detail

Event Timeline

mingmingl created this revision.Aug 2 2022, 11:11 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 2 2022, 11:11 PM
mingmingl requested review of this revision.Aug 2 2022, 11:11 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 2 2022, 11:11 PM

DO NOT SEND

Diffbase to show an alternative approach

add another test case test2 in llvm/test/CodeGen/AArch64/aarch64-pmull2.ll

mingmingl updated this revision to Diff 453220.Aug 17 2022, 1:03 AM

simplify test case a little bit

mingmingl updated this revision to Diff 453226.Aug 17 2022, 1:08 AM

simplify test case, by removing unnecessary xor, etc

mingmingl retitled this revision from [NFC][AArch64] Precommit test for a tablegen pattern for aarch64_neon_pmull64 to [NFC][AArch64] Precommit test to optimize instruction selection for aarch64_neon_pmull64 intrinsic..
mingmingl edited the summary of this revision. (Show Details)
mingmingl added reviewers: efriedma, dmgreen.

Update revision title and summary. Use same comments for NFC and actual patch, so only assembly diff is shown in phabricator UI (not comment diff)

From discussion with colleagues I should probably ask for review for both pre-commit tests and the actual change in the first place to ensure the quality of new test cases (e.g., not overly sensitive to changes that might make it brittle/likely to fail for unrelated reasons, not insufficiently minimized so that it's hard to maintain/read/possibly and more likely to fail for unrelated reasons, have good readability in terms of naming, etc)

So do it now and will remember to ask reviews for both in the future. Hope it's not too late!

dmgreen accepted this revision.Aug 19 2022, 10:39 AM

Oh yeah - LGTM

This revision is now accepted and ready to land.Aug 19 2022, 10:39 AM
This revision was landed with ongoing or failed builds.Aug 19 2022, 1:18 PM
This revision was automatically updated to reflect the committed changes.