[Scalarizer] Do not insert instructions between PHI nodes and debug intrinsics.

The scalarizer pass seems to be inserting instructions in-between PHI nodes or debug intrinsics that end up staying at the end of the pass, resulting in malformed IR and violating assumptions.

This patch adds a check to make sure the `extractelement` instructions that it adds are correctly placed after all PHI nodes and debug intrinsics.

Patch by vettoreldaniele.

Reviewed By: bjope

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

llvm/lib/Transforms/Scalar/Scalarizer.cpp

@@ -66,6 +66,15 @@ static cl::opt<bool>
 
 namespace {
 
+BasicBlock::iterator skipPastPhiNodesAndDbg(BasicBlock::iterator Itr) {
+  BasicBlock *BB = Itr->getParent();
+  if (isa<PHINode>(Itr))
+    Itr = BB->getFirstInsertionPt();
+  if (Itr != BB->end())
+    Itr = skipDebugIntrinsics(Itr);
+  return Itr;
+}
+
 // Used to store the scattered form of a vector.
 using ValueVector = SmallVector<Value *, 8>;
 
@@ -371,10 +380,11 @@ Scatterer ScalarizerVisitor::scatter(Instruction *Point, Value *V) {
       return Scatterer(Point->getParent(), Point->getIterator(),
                        UndefValue::get(V->getType()));
     // Put the scattered form of an instruction directly after the
-    // instruction.
+    // instruction, skipping over PHI nodes and debug intrinsics.
     BasicBlock *BB = VOp->getParent();
-    return Scatterer(BB, std::next(BasicBlock::iterator(VOp)),
-                     V, &Scattered[V]);
+    return Scatterer(
+        BB, skipPastPhiNodesAndDbg(std::next(BasicBlock::iterator(VOp))), V,
+        &Scattered[V]);
   }
   // In the fallback case, just put the scattered before Point and
   // keep the result local to Point.

llvm/test/Transforms/Scalarizer/dbg-invariant.ll

@@ -0,0 +1,64 @@
+; RUN: opt -strip-debug -passes=scalarizer -S < %s | FileCheck %s
+; RUN: opt -passes=scalarizer -S < %s | FileCheck %s
+
+; This input caused the scalarizer to violate a debug info
+; invariance by using the wrong insertion point.
+
+; CHECK: %0 = load <8 x i16>
+
+; CHECK: %.i0 = extractelement <8 x i16> %0, i32 0
+; CHECK-NEXT: %.i01 = add i16 %.i0, 28690
+; CHECK: %.i1 = extractelement <8 x i16> %0, i32 1
+; CHECK-NEXT: %.i12 = add i16 %.i1, 28690
+; CHECK: %.i2 = extractelement <8 x i16> %0, i32 2
+; CHECK-NEXT: %.i23 = add i16 %.i2, 28690
+; CHECK: %.i3 = extractelement <8 x i16> %0, i32 3
+; CHECK-NEXT: %.i34 = add i16 %.i3, 28690
+; CHECK: %.i4 = extractelement <8 x i16> %0, i32 4
+; CHECK-NEXT: %.i45 = add i16 %.i4, 28690
+; CHECK: %.i5 = extractelement <8 x i16> %0, i32 5
+; CHECK-NEXT: %.i56 = add i16 %.i5, 28690
+; CHECK: %.i6 = extractelement <8 x i16> %0, i32 6
+; CHECK-NEXT: %.i67 = add i16 %.i6, 28690
+; CHECK: %.i7 = extractelement <8 x i16> %0, i32 7
+; CHECK-NEXT: = add i16 %.i7, 28690
+
+@d = external global [8 x i16], align 1
+@e = external global i16, align 1
+
+; Function Attrs: nofree norecurse nounwind
+define dso_local void @foo() local_unnamed_addr #0 !dbg !7 {
+entry:
+  %0 = load <8 x i16>, <8 x i16>* bitcast ([8 x i16]* @d to <8 x i16>*), align 1
+  call void @llvm.dbg.value(metadata i16 0, metadata !11, metadata !DIExpression()), !dbg !13
+  %1 = add <8 x i16> %0, <i16 28690, i16 28690, i16 28690, i16 28690, i16 28690, i16 28690, i16 28690, i16 28690>, !dbg !13
+  store <8 x i16> %1, <8 x i16>* bitcast ([8 x i16]* @d to <8 x i16>*), align 1, !dbg !13
+  %2 = extractelement <8 x i16> %1, i32 7, !dbg !13
+  store i16 %2, i16* @e, align 1, !dbg !13
+  ret void
+}
+
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.value(metadata, metadata, metadata) #1
+
+attributes #0 = { nofree norecurse nounwind }
+attributes #1 = { nounwind readnone speculatable willreturn }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 11.0.0 ", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "foo.c", directory: "/")
+!2 = !{}
+!3 = !{i32 7, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 1}
+!6 = !{!"clang version 11.0.0 "}
+!7 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 4, type: !8, scopeLine: 4, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !10)
+!8 = !DISubroutineType(types: !9)
+!9 = !{null}
+!10 = !{!11}
+!11 = !DILocalVariable(name: "i", scope: !7, file: !1, line: 5, type: !12)
+!12 = !DIBasicType(name: "int", size: 16, encoding: DW_ATE_signed)
+!13 = !DILocation(line: 0, scope: !7)

llvm/test/Transforms/Scalarizer/phi-order.ll

@@ -0,0 +1,29 @@
+; RUN: opt %s -passes='function(scalarizer)' -S -o - | FileCheck %s
+
+; This input caused the scalarizer to insert non-PHI nodes
+; in between PHI nodes (%1 and %2).
+
+define <3 x float> @func(i32 %inval) {
+.entry:
+  br label %0
+
+0:                                                ; preds = %3, %.entry
+; CHECK: %.i01 = phi float [ 0.000000e+00, %.entry ], [ %.i01, %3 ]
+; CHECK-NEXT: %.i12 = phi float [ 0.000000e+00, %.entry ], [ %.i12, %3 ]
+; CHECK-NEXT: %.i23 = phi float [ 0.000000e+00, %.entry ], [ %.i23, %3 ]
+; CHECK-NEXT: %1 = phi float [ 1.000000e+00, %.entry ], [ 2.000000e+00, %3 ]
+; CHECK-NEXT: %.upto0 = insertelement <3 x float> poison, float %.i01, i32 0
+; CHECK-NEXT: %.upto1 = insertelement <3 x float> %.upto0, float %.i12, i32 1
+; CHECK-NEXT: %2 = insertelement <3 x float> %.upto1, float %.i23, i32 2
+  %1 = phi <3 x float> [ <float 0.0, float 0.0, float 0.0>, %.entry], [ %1, %3 ]
+  %2 = phi float [ 1.0, %.entry], [ 2.0, %3 ]
+  br label %3
+
+3:                                                ; preds = %0
+  %cond = icmp eq i32 %inval, 0
+  br i1 %cond, label %0, label %exit
+
+exit:
+  ret <3 x float>  %1
+}
+