[llvm] 4e1a068 - [SLP]Fix non-determinism in PHI sorting.

[Alexey Bataev via llvm-commits]

Author: Alexey Bataev
Date: 2021-07-06T08:45:45-07:00
New Revision: 4e1a0684f13d833e6ddec94d9f7738b0a004e4c1

URL: https://github.com/llvm/llvm-project/commit/4e1a0684f13d833e6ddec94d9f7738b0a004e4c1
DIFF: https://github.com/llvm/llvm-project/commit/4e1a0684f13d833e6ddec94d9f7738b0a004e4c1.diff

LOG: [SLP]Fix non-determinism in PHI sorting.

Compare type IDs and DFS numbering for basic block instead of addresses
to fix non-determinism.

Differential Revision: https://reviews.llvm.org/D105031

Added: 
    

Modified: 
    llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
    llvm/test/Transforms/SLPVectorizer/X86/remark_unsupported.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index f687cce8d0e2..2362a7417892 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -8318,7 +8318,7 @@ bool SLPVectorizerPass::vectorizeChainsInBlock(BasicBlock *BB, BoUpSLP &R) {
       // No need to analyze deleted, vectorized and non-vectorizable
       // instructions.
       if (!VisitedInstrs.count(P) && !R.isDeleted(P) &&
-          !P->getType()->isVectorTy())
+          isValidElementType(P->getType()))
         Incoming.push_back(P);
     }
 
@@ -8346,10 +8346,15 @@ bool SLPVectorizerPass::vectorizeChainsInBlock(BasicBlock *BB, BoUpSLP &R) {
     }
 
     // Sort by type, parent, operands.
-    stable_sort(Incoming, [&PHIToOpcodes](Value *V1, Value *V2) {
-      if (V1->getType() < V2->getType())
+    stable_sort(Incoming, [this, &PHIToOpcodes](Value *V1, Value *V2) {
+      assert(isValidElementType(V1->getType()) &&
+             isValidElementType(V2->getType()) &&
+             "Expected vectorizable types only.");
+      // It is fine to compare type IDs here, since we expect only vectorizable
+      // types, like ints, floats and pointers, we don't care about other type.
+      if (V1->getType()->getTypeID() < V2->getType()->getTypeID())
         return true;
-      if (V1->getType() > V2->getType())
+      if (V1->getType()->getTypeID() > V2->getType()->getTypeID())
         return false;
       ArrayRef<Value *> Opcodes1 = PHIToOpcodes[V1];
       ArrayRef<Value *> Opcodes2 = PHIToOpcodes[V2];
@@ -8363,10 +8368,15 @@ bool SLPVectorizerPass::vectorizeChainsInBlock(BasicBlock *BB, BoUpSLP &R) {
           continue;
         if (auto *I1 = dyn_cast<Instruction>(Opcodes1[I]))
           if (auto *I2 = dyn_cast<Instruction>(Opcodes2[I])) {
-            if (I1->getParent() < I2->getParent())
-              return true;
-            if (I1->getParent() > I2->getParent())
-              return false;
+            DomTreeNodeBase<BasicBlock> *NodeI1 = DT->getNode(I1->getParent());
+            DomTreeNodeBase<BasicBlock> *NodeI2 = DT->getNode(I2->getParent());
+            assert(NodeI1 && "Should only process reachable instructions");
+            assert(NodeI2 && "Should only process reachable instructions");
+            assert((NodeI1 == NodeI2) ==
+                       (NodeI1->getDFSNumIn() == NodeI2->getDFSNumIn()) &&
+                   "Different nodes should have 
diff erent DFS numbers");
+            if (NodeI1 != NodeI2)
+              return NodeI1->getDFSNumIn() < NodeI2->getDFSNumIn();
             InstructionsState S = getSameOpcode({I1, I2});
             if (S.getOpcode())
               continue;

diff  --git a/llvm/test/Transforms/SLPVectorizer/X86/remark_unsupported.ll b/llvm/test/Transforms/SLPVectorizer/X86/remark_unsupported.ll
index afc08aa4fb27..5511827332a2 100644
--- a/llvm/test/Transforms/SLPVectorizer/X86/remark_unsupported.ll
+++ b/llvm/test/Transforms/SLPVectorizer/X86/remark_unsupported.ll
@@ -3,7 +3,7 @@
 ; RUN: FileCheck --input-file=%t --check-prefix=YAML %s
 
 ; This type is not supported by SLP
-define void @test(x86_fp80* %i1, x86_fp80* %i2, x86_fp80* %o) {
+define i1 @test(x86_fp80* %i1, x86_fp80* %i2) {
 ; CHECK-LABEL: @test(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[I1_0:%.*]] = load x86_fp80, x86_fp80* [[I1:%.*]], align 16
@@ -19,10 +19,8 @@ define void @test(x86_fp80* %i1, x86_fp80* %i2, x86_fp80* %o) {
 ; CHECK:       end:
 ; CHECK-NEXT:    [[PHI0:%.*]] = phi x86_fp80 [ [[I1_0]], [[ENTRY:%.*]] ], [ [[I2_0]], [[THEN]] ]
 ; CHECK-NEXT:    [[PHI1:%.*]] = phi x86_fp80 [ [[I1_1]], [[ENTRY]] ], [ [[I2_1]], [[THEN]] ]
-; CHECK-NEXT:    store x86_fp80 [[PHI0]], x86_fp80* [[O:%.*]], align 16
-; CHECK-NEXT:    [[O_GEP1:%.*]] = getelementptr inbounds x86_fp80, x86_fp80* [[O]], i64 1
-; CHECK-NEXT:    store x86_fp80 [[PHI1]], x86_fp80* [[O_GEP1]], align 16
-; CHECK-NEXT:    ret void
+; CHECK-NEXT:    [[RES:%.*]] = fcmp oeq x86_fp80 [[PHI0]], [[PHI1]]
+; CHECK-NEXT:    ret i1 [[RES]]
 ;
 entry:
   %i1.0 = load x86_fp80, x86_fp80* %i1, align 16
@@ -37,11 +35,8 @@ then:
   br label %end
 end:
   %phi0 = phi x86_fp80 [ %i1.0, %entry ], [ %i2.0, %then ]
-
   %phi1 = phi x86_fp80 [ %i1.1, %entry ], [ %i2.1, %then ]
-  store x86_fp80 %phi0, x86_fp80* %o, align 16
-  %o.gep1 = getelementptr inbounds x86_fp80, x86_fp80* %o, i64 1
-  store x86_fp80 %phi1, x86_fp80* %o.gep1, align 16
+  %res = fcmp oeq x86_fp80 %phi0, %phi1
   ; YAML:      Pass:            slp-vectorizer
   ; YAML-NEXT: Name:            UnsupportedType
   ; YAML-NEXT: Function:        test
@@ -49,5 +44,5 @@ end:
   ; YAML-NEXT:   - String:          'Cannot SLP vectorize list: type '
   ; YAML-NEXT:   - String:          x86_fp80 is unsupported by vectorizer
 
-  ret void
+  ret i1 %res
 }