[llvm] SLPVectorizer comparator reordering (PR #84969)

[David Blaikie via llvm-commits]

https://github.com/dwblaikie created https://github.com/llvm/llvm-project/pull/84969

Based on feedback in #79321, the ordering might be better changed from:

  instructions, constants, undef, other values

to:

  instructions, undefs, constants, other values

So here's a patch that makes that change - though two tests fail
(discussed here:
https://github.com/llvm/llvm-project/issues/79321#issuecomment-1986694793
)

test/Transforms/SLPVectorizer/X86/reused-undefs.ll: https://www.diffchecker.com/V7BSldjt/
test/Transforms/SLPVectorizer/X86/phi-undef-input.ll: https://www.diffchecker.com/pctnAozF/

So something needs to be done to understand whether those are
improvements, regressions, or something else - whether further
improvements are needed to account for the change in ordering.


>From 3e81bdefe5d510d2fa226bf5729527851adeda0d Mon Sep 17 00:00:00 2001
From: David Blaikie <dblaikie at gmail.com>
Date: Tue, 12 Mar 2024 18:37:37 +0000
Subject: [PATCH] SLPVectorizer comparator reordering

Based on feedback in #79321, the ordering might be better changed from:

  instructions, constants, undef, other values

to:

  instructions, undefs, constants, other values

So here's a patch that makes that change - though two tests fail
(discussed here:
https://github.com/llvm/llvm-project/issues/79321#issuecomment-1986694793
)

test/Transforms/SLPVectorizer/X86/reused-undefs.ll: https://www.diffchecker.com/V7BSldjt/
test/Transforms/SLPVectorizer/X86/phi-undef-input.ll: https://www.diffchecker.com/pctnAozF/

So something needs to be done to understand whether those are
improvements, regressions, or something else - whether further
improvements are needed to account for the change in ordering.
---
 .../Transforms/Vectorize/SLPVectorizer.cpp    | 42 +++++++++----------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index b8b67609d755fd..2ebcaeb33bd52f 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -16641,9 +16641,20 @@ bool SLPVectorizerPass::vectorizeChainsInBlock(BasicBlock *BB, BoUpSLP &R) {
           return false;
       }
       {
-        // Non-undef constants come next.
-        bool C1 = isa<Constant>(Opcodes1[I]) && !isa<UndefValue>(Opcodes1[I]);
-        bool C2 = isa<Constant>(Opcodes2[I]) && !isa<UndefValue>(Opcodes2[I]);
+        // Sort by undefs next.
+        bool U1 = isa<UndefValue>(Opcodes1[I]);
+        bool U2 = isa<UndefValue>(Opcodes2[I]);
+        if (U1 && U2)
+          continue;
+        if (U1)
+          return true;
+        if (U2)
+          return false;
+      }
+      {
+        // Sort by non-undef constants next.
+        bool C1 = isa<Constant>(Opcodes1[I]);
+        bool C2 = isa<Constant>(Opcodes2[I]);
         if (C1 && C2)
           continue;
         if (C1)
@@ -16651,28 +16662,17 @@ bool SLPVectorizerPass::vectorizeChainsInBlock(BasicBlock *BB, BoUpSLP &R) {
         if (C2)
           return false;
       }
-      bool U1 = isa<UndefValue>(Opcodes1[I]);
-      bool U2 = isa<UndefValue>(Opcodes2[I]);
       {
-        // Non-constant non-instructions come next.
-        if (!U1 && !U2) {
-          auto ValID1 = Opcodes1[I]->getValueID();
-          auto ValID2 = Opcodes2[I]->getValueID();
-          if (ValID1 == ValID2)
-            continue;
-          if (ValID1 < ValID2)
-            return true;
-          if (ValID1 > ValID2)
-            return false;
-        }
-        if (!U1)
+        // Sort the rest by ValueID.
+        auto ValID1 = Opcodes1[I]->getValueID();
+        auto ValID2 = Opcodes2[I]->getValueID();
+        if (ValID1 == ValID2)
+          continue;
+        if (ValID1 < ValID2)
           return true;
-        if (!U2)
+        if (ValID1 > ValID2)
           return false;
       }
-      // Undefs come last.
-      assert(U1 && U2 && "The only thing left should be undef & undef.");
-      continue;
     }
     return false;
   };