Add SD matchers and unit test coverage for ISD::VECTOR_SHUFFLE #119592

AidanGoldfarb · 2024-12-11T17:16:36Z

This PR resolves #118845. I aimed to mirror the implementation m_Shuffle() in PatternMatch.h.

Added struct m_Mask to match masks (ArrayRef<int>)
Added two m_Shuffle functions. One to match independently of mask, and one to match considering mask.
Added struct SDShuffle_match to match ISD::VECTOR_SHUFFLE considering mask

Added matchVecShuffle test, which tests the behavior of both m_Shuffle() functions

I am not sure if my test coverage is complete. I am not sure how to test a false match, simply test against a different instruction? Other tests , such as for VSelect, test against Select. I am not sure if there is an analogous instruction to compare against for VECTOR_SHUFFLE. I would appreciate some pointers in this area. In general, please liberally critique this PR!

llvm/include/llvm/CodeGen/SDPatternMatch.h

llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp

…Mask.match() with a std::equal(mask,i->getMask(). changed matchVecShuffle test to not initialize Mask. Also added a test to compare contents of MaskData to Mask.

github-actions · 2024-12-12T19:15:58Z

✅ With the latest revision this PR passed the C/C++ code formatter.

llvm/include/llvm/CodeGen/SDPatternMatch.h

llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp

llvm/include/llvm/CodeGen/SDPatternMatch.h

…res mask as &arrayref. Snd matches specific contents as arrayref. Removed m_mask, updated tests

llvm/include/llvm/CodeGen/SDPatternMatch.h

llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp

llvm/include/llvm/CodeGen/SDPatternMatch.h

…equal(). Readded cmp for capture test. Removed const for ArrayRef. Correctly capture arrayref in m_Shuffle varient. Changed m_Value(V) to m_Value, as vec contents not needed.

llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp

llvm/include/llvm/CodeGen/SDPatternMatch.h

llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp

mshockwave

LGTM w/ minor comments.

llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp

mshockwave · 2024-12-20T17:25:32Z

Feel free to merge it when the merge conflicts are fixed.

RKSimon · 2024-12-20T17:46:49Z

Thanks @AidanGoldfarb

topperc · 2024-12-20T20:09:31Z

I kind of think we should have separate m_Shuffle and m_Mask/m_SpecificMask like IR. That will allow more m_*Mask to be added in the future without needing to duplicate all of the code. For example, we may want one that takes a function_ref to do custom matching. The IR match function for m_Mask/m_SpecificMask takes an ArrayRef<int> Mask as its argument.

RKSimon · 2024-12-23T15:20:44Z

@AidanGoldfarb Do you think we could keep closer to the PatternMatch approach of m_Shuffle / m_Mask / m_SpecificMask (and m_SplatOrPoisonMask / m_ZeroMask .....) ?

AidanGoldfarb · 2024-12-24T20:21:01Z

I just pushed some changes that mirror the PatternMatch approach as closely as possible. This includes
m_Mask and m_SpecificMask, with room to add things like m_ZeroMask. I did not add m_ZeroMask or m_SplatOrPoisonMask but would be happy to do it if there is a use case.

llvm/include/llvm/CodeGen/SDPatternMatch.h

llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp

llvm/include/llvm/CodeGen/SDPatternMatch.h

mshockwave

LGTM
@topperc do you have any other comments?

topperc

LGTM

Aidan added 2 commits December 11, 2024 12:13

Added SD matchers and unit test coverage for ISD::VECTOR_SHUFFLE

4649aa0

Cleaned up code

1b3dd6f

AidanGoldfarb marked this pull request as ready for review December 11, 2024 21:55

AidanGoldfarb requested a review from RKSimon December 11, 2024 21:56

AidanGoldfarb changed the title ~~Added SD matchers and unit test coverage for ISD::VECTOR_SHUFFLE~~ Add SD matchers and unit test coverage for ISD::VECTOR_SHUFFLE Dec 11, 2024

RKSimon requested a review from mshockwave December 12, 2024 14:02

AidanGoldfarb requested a review from justinfargnoli December 12, 2024 16:40

mshockwave reviewed Dec 12, 2024

View reviewed changes

llvm/include/llvm/CodeGen/SDPatternMatch.h Show resolved Hide resolved

llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp Outdated Show resolved Hide resolved

llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp Outdated Show resolved Hide resolved

changed SDShuffle_match to store Mask directly. Replaced the call to …

65ab88c

…Mask.match() with a std::equal(mask,i->getMask(). changed matchVecShuffle test to not initialize Mask. Also added a test to compare contents of MaskData to Mask.

formatting fix

c577f7b

mshockwave reviewed Dec 12, 2024

View reviewed changes

proposed changes. Split functionality of m_shuffle. Fst varient captu…

b365d5b

…res mask as &arrayref. Snd matches specific contents as arrayref. Removed m_mask, updated tests

AidanGoldfarb requested a review from mshockwave December 14, 2024 19:49

mshockwave reviewed Dec 15, 2024

View reviewed changes

Updated element-wise arrayref cmps to not use overridden == but std::…

c77ac06

…equal(). Readded cmp for capture test. Removed const for ArrayRef. Correctly capture arrayref in m_Shuffle varient. Changed m_Value(V) to m_Value, as vec contents not needed.

RKSimon reviewed Dec 15, 2024

View reviewed changes

llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp Outdated Show resolved Hide resolved

added some expect_false tests for mask capture and matching

d818fd5

AidanGoldfarb requested a review from mshockwave December 15, 2024 18:05

mshockwave reviewed Dec 16, 2024

View reviewed changes

llvm/include/llvm/CodeGen/SDPatternMatch.h Outdated Show resolved Hide resolved

llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp Outdated Show resolved Hide resolved

Aidan added 2 commits December 16, 2024 20:30

only capture mask if ops match. reordered tests

e9987c6

remove space

350d538

mshockwave approved these changes Dec 19, 2024

View reviewed changes

llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp Outdated Show resolved Hide resolved

llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp Outdated Show resolved Hide resolved

smallvec->const std::array.formatting fix

98fdeeb

Merge branch 'main' into sdpatternmatch

e5c3bc2

Accidental '}'removed

860bf81

refactored to more closley follow the IR PatternMatch implementation

53497e7

AidanGoldfarb requested review from RKSimon and topperc December 24, 2024 20:22

mshockwave reviewed Dec 24, 2024

View reviewed changes

llvm/include/llvm/CodeGen/SDPatternMatch.h Outdated Show resolved Hide resolved

llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp Outdated Show resolved Hide resolved

llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp Outdated Show resolved Hide resolved

Aidan added 2 commits December 25, 2024 10:35

ArrayRef passed by value. Added test, may remove another later.

80e64f6

Removed less meaningufl test

dea262c

topperc reviewed Dec 27, 2024

View reviewed changes

llvm/include/llvm/CodeGen/SDPatternMatch.h Outdated Show resolved Hide resolved

llvm/include/llvm/CodeGen/SDPatternMatch.h Outdated Show resolved Hide resolved

Aidan added 2 commits December 30, 2024 14:20

m_mask by ref, m_specmask by value

75ba05b

remove extra mask

df90470

mshockwave approved these changes Jan 6, 2025

View reviewed changes

topperc approved these changes Jan 6, 2025

View reviewed changes

AidanGoldfarb merged commit f3bc8c3 into llvm:main Jan 6, 2025
8 checks passed

Add SD matchers and unit test coverage for ISD::VECTOR_SHUFFLE #119592

Add SD matchers and unit test coverage for ISD::VECTOR_SHUFFLE #119592

Uh oh!

Conversation

AidanGoldfarb commented Dec 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Dec 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mshockwave left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mshockwave commented Dec 20, 2024

Uh oh!

RKSimon commented Dec 20, 2024

Uh oh!

topperc commented Dec 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RKSimon commented Dec 23, 2024

Uh oh!

AidanGoldfarb commented Dec 24, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mshockwave left a comment

Choose a reason for hiding this comment

Uh oh!

topperc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AidanGoldfarb commented Dec 11, 2024 •

edited

Loading

github-actions bot commented Dec 12, 2024 •

edited

Loading

mshockwave left a comment •

edited

Loading

topperc commented Dec 20, 2024 •

edited

Loading