Skip to content

[SLP][REVEC] reorderNodeWithReuses should not be called if all users of a TreeEntry are ShuffleVectorInst. #118260

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

Merged
merged 4 commits into from
Dec 3, 2024

Conversation

HanKuanChen
Copy link
Contributor

No description provided.

Copy link

graphite-app bot commented Dec 2, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “FP Bundles” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

@llvmbot
Copy link
Member

llvmbot commented Dec 2, 2024

@llvm/pr-subscribers-vectorizers

@llvm/pr-subscribers-llvm-transforms

Author: Han-Kuan Chen (HanKuanChen)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/118260.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp (+17)
  • (modified) llvm/test/Transforms/SLPVectorizer/revec.ll (+34)
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 04755102643364..74f5df11ed6a9e 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -6073,6 +6073,23 @@ void BoUpSLP::reorderTopToBottom() {
                                      TE->Scalars.size();
                         }) &&
                  "All users must be of VF size.");
+          if (SLPReVec) {
+            assert(SLPReVec && "Only supported by REVEC.");
+            // ShuffleVectorInst does not do reorderOperands (and it should not
+            // because ShuffleVectorInst supports only a limited set of
+            // patterns). Only do reorderNodeWithReuses if all of the users are
+            // not ShuffleVectorInst.
+            if (all_of(TE->UserTreeIndices, [&](const EdgeInfo &EI) {
+                  return isa<ShuffleVectorInst>(EI.UserTE->getMainOp());
+                }))
+              continue;
+            assert(all_of(TE->UserTreeIndices,
+                          [&](const EdgeInfo &EI) {
+                            return !isa<ShuffleVectorInst>(
+                                EI.UserTE->getMainOp());
+                          }) &&
+                   "Does not know how to reorder.");
+          }
           // Update ordering of the operands with the smaller VF than the given
           // one.
           reorderNodeWithReuses(*TE, Mask);
diff --git a/llvm/test/Transforms/SLPVectorizer/revec.ll b/llvm/test/Transforms/SLPVectorizer/revec.ll
index b160c0174c0a76..ce13f478d38116 100644
--- a/llvm/test/Transforms/SLPVectorizer/revec.ll
+++ b/llvm/test/Transforms/SLPVectorizer/revec.ll
@@ -447,3 +447,37 @@ for.end.loopexit:
   store <4 x i32> %4, ptr %out2, align 4
   ret void
 }
+
+define void @test14(<8 x i1> %0) {
+; CHECK-LABEL: @test14(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TMP1:%.*]] = call <16 x i1> @llvm.vector.insert.v16i1.v8i1(<16 x i1> poison, <8 x i1> [[TMP0:%.*]], i64 0)
+; CHECK-NEXT:    [[TMP2:%.*]] = shufflevector <16 x i1> [[TMP1]], <16 x i1> poison, <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7>
+; CHECK-NEXT:    [[TMP3:%.*]] = sext <16 x i1> [[TMP2]] to <16 x i16>
+; CHECK-NEXT:    [[TMP4:%.*]] = shufflevector <16 x i16> [[TMP3]], <16 x i16> poison, <32 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 8, i32 9, i32 10, i32 11, i32 12, i32 13, i32 14, i32 15, i32 8, i32 9, i32 10, i32 11, i32 12, i32 13, i32 14, i32 15>
+; CHECK-NEXT:    [[TMP5:%.*]] = shufflevector <16 x i16> [[TMP3]], <16 x i16> poison, <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 8, i32 9, i32 10, i32 11, i32 12, i32 13, i32 14, i32 15>
+; CHECK-NEXT:    [[TMP6:%.*]] = shufflevector <16 x i16> [[TMP3]], <16 x i16> poison, <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 8, i32 9, i32 10, i32 11, i32 12, i32 13, i32 14, i32 15, i32 4, i32 5, i32 6, i32 7>
+; CHECK-NEXT:    br label [[FOR_END_LOOPEXIT:%.*]]
+; CHECK:       for.end.loopexit:
+; CHECK-NEXT:    [[TMP7:%.*]] = phi <16 x i16> [ [[TMP6]], [[ENTRY:%.*]] ]
+; CHECK-NEXT:    [[TMP8:%.*]] = call <4 x i16> @llvm.vector.extract.v4i16.v16i16(<16 x i16> [[TMP7]], i64 12)
+; CHECK-NEXT:    [[OR0:%.*]] = or <4 x i16> [[TMP8]], zeroinitializer
+; CHECK-NEXT:    ret void
+;
+entry:
+  %sext0 = sext <8 x i1> %0 to <8 x i16>
+  %sext1 = sext <8 x i1> %0 to <8 x i16>
+  %1 = shufflevector <8 x i16> %sext0, <8 x i16> zeroinitializer, <4 x i32> <i32 0, i32 1, i32 2, i32 3>
+  %2 = shufflevector <8 x i16> %sext0, <8 x i16> zeroinitializer, <4 x i32> <i32 4, i32 5, i32 6, i32 7>
+  %3 = shufflevector <8 x i16> %sext1, <8 x i16> zeroinitializer, <4 x i32> <i32 0, i32 1, i32 2, i32 3>
+  %4 = shufflevector <8 x i16> %sext1, <8 x i16> zeroinitializer, <4 x i32> <i32 4, i32 5, i32 6, i32 7>
+  br label %for.end.loopexit
+
+for.end.loopexit:
+  %phi0 = phi <4 x i16> [ %1, %entry ]
+  %phi1 = phi <4 x i16> [ %2, %entry ]
+  %phi2 = phi <4 x i16> [ %3, %entry ]
+  %phi3 = phi <4 x i16> [ %4, %entry ]
+  %or0 = or <4 x i16> %phi1, zeroinitializer
+  ret void
+}

Copy link

github-actions bot commented Dec 2, 2024

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

@HanKuanChen HanKuanChen merged commit f71ea4b into llvm:main Dec 3, 2024
8 checks passed
@HanKuanChen HanKuanChen deleted the slp-revec-reorderNodeWithReuses branch December 3, 2024 01:04
@llvm-ci
Copy link
Collaborator

llvm-ci commented Dec 3, 2024

LLVM Buildbot has detected a new failure on builder libc-x86_64-debian-gcc-fullbuild-dbg running on libc-x86_64-debian-fullbuild while building llvm at step 4 "annotate".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/131/builds/11538

Here is the relevant piece of the build log for the reference
Step 4 (annotate) failure: 'python ../llvm-zorg/zorg/buildbot/builders/annotated/libc-linux.py ...' (failure)
...
[       OK ] LlvmLibcMlockTest.MLockAll (1 ms)
Ran 5 tests.  PASS: 5  FAIL: 0
[936/1103] Running unit test libc.test.src.sys.mman.linux.shm_test
[==========] Running 2 tests from 1 test suite.
[ RUN      ] LlvmLibcShmTest.Basic
[       OK ] LlvmLibcShmTest.Basic (151 us)
[ RUN      ] LlvmLibcShmTest.NameConversion
[       OK ] LlvmLibcShmTest.NameConversion (52 us)
Ran 2 tests.  PASS: 2  FAIL: 0
[937/1103] Running unit test libc.test.src.sys.mman.linux.process_mrelease_test
FAILED: projects/libc/test/src/sys/mman/linux/CMakeFiles/libc.test.src.sys.mman.linux.process_mrelease_test /home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-gcc-fullbuild-dbg/build/projects/libc/test/src/sys/mman/linux/CMakeFiles/libc.test.src.sys.mman.linux.process_mrelease_test 
cd /home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-gcc-fullbuild-dbg/build/projects/libc/test/src/sys/mman/linux && /home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-gcc-fullbuild-dbg/build/projects/libc/test/src/sys/mman/linux/libc.test.src.sys.mman.linux.process_mrelease_test.__build__
[==========] Running 3 tests from 1 test suite.
[ RUN      ] LlvmLibcProcessMReleaseTest.NoError
/home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-gcc-fullbuild-dbg/llvm-project/libc/test/src/sys/mman/linux/process_mrelease_test.cpp:44: FAILURE
Failed to match LIBC_NAMESPACE::process_mrelease(pidfd, 0) against Succeeds().
Expected return value to be equal to 0 but got -1.
Expected errno to be equal to "Success" but got "No such process".
[  FAILED  ] LlvmLibcProcessMReleaseTest.NoError
[ RUN      ] LlvmLibcProcessMReleaseTest.ErrorNotKilled
[       OK ] LlvmLibcProcessMReleaseTest.ErrorNotKilled (128 us)
[ RUN      ] LlvmLibcProcessMReleaseTest.ErrorNonExistingPidfd
[       OK ] LlvmLibcProcessMReleaseTest.ErrorNonExistingPidfd (7 us)
Ran 3 tests.  PASS: 2  FAIL: 1
[938/1103] Running unit test libc.test.src.sys.random.linux.getrandom_test
[==========] Running 4 tests from 1 test suite.
[ RUN      ] LlvmLibcGetRandomTest.InvalidFlag
[       OK ] LlvmLibcGetRandomTest.InvalidFlag (4 us)
[ RUN      ] LlvmLibcGetRandomTest.InvalidBuffer
[       OK ] LlvmLibcGetRandomTest.InvalidBuffer (6 us)
[ RUN      ] LlvmLibcGetRandomTest.ReturnsSize
[       OK ] LlvmLibcGetRandomTest.ReturnsSize (17 us)
[ RUN      ] LlvmLibcGetRandomTest.CheckValue
[       OK ] LlvmLibcGetRandomTest.CheckValue (6 us)
Ran 4 tests.  PASS: 4  FAIL: 0
[939/1103] Running unit test libc.test.src.sys.random.linux.getrandom_test.__NO_MISC_MATH_BASIC_OPS_OPT
[==========] Running 4 tests from 1 test suite.
[ RUN      ] LlvmLibcGetRandomTest.InvalidFlag
[       OK ] LlvmLibcGetRandomTest.InvalidFlag (4 us)
[ RUN      ] LlvmLibcGetRandomTest.InvalidBuffer
[       OK ] LlvmLibcGetRandomTest.InvalidBuffer (6 us)
[ RUN      ] LlvmLibcGetRandomTest.ReturnsSize
[       OK ] LlvmLibcGetRandomTest.ReturnsSize (17 us)
[ RUN      ] LlvmLibcGetRandomTest.CheckValue
[       OK ] LlvmLibcGetRandomTest.CheckValue (5 us)
Ran 4 tests.  PASS: 4  FAIL: 0
[940/1103] Running unit test libc.test.src.sys.random.linux.getrandom_test.__NO_FMA_OPT
[==========] Running 4 tests from 1 test suite.
[ RUN      ] LlvmLibcGetRandomTest.InvalidFlag

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants