[llvm] db05f2e - [Scalarizer] Centralize instruction DCE
Roman Lebedev via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 6 15:13:27 PDT 2020
Author: Roman Lebedev
Date: 2020-07-07T01:12:51+03:00
New Revision: db05f2e34a5e9380ddcc199d6687531108d795e4
URL: https://github.com/llvm/llvm-project/commit/db05f2e34a5e9380ddcc199d6687531108d795e4
DIFF: https://github.com/llvm/llvm-project/commit/db05f2e34a5e9380ddcc199d6687531108d795e4.diff
LOG: [Scalarizer] Centralize instruction DCE
As reported in https://reviews.llvm.org/D83101#2133062
the new visitInsertElementInst()/visitExtractElementInst() functionality
is causing miscompiles (previously-crashing test added)
It is due to the fact how the infra of Scalarizer is dealing with DCE,
it was not updated or was it ready for such scalar value forwarding.
It always assumed that the moment we "scalarized" something,
it can go away, and did so with prejudice.
But that is no longer safe/okay to do.
Instead, let's prevent it from ever shooting itself into foot,
and let's just accumulate the instructions-to-be-deleted
in a vector, and collectively cleanup (those that are *actually* dead)
them all at the end.
All existing tests are not reporting any new garbage leftovers,
but maybe it's test coverage issue.
Added:
Modified:
llvm/lib/Transforms/Scalar/Scalarizer.cpp
llvm/test/Transforms/Scalarizer/basic.ll
llvm/test/Transforms/Scalarizer/crash-bug.ll
llvm/test/Transforms/Scalarizer/phi-unreachable-pred.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Scalar/Scalarizer.cpp b/llvm/lib/Transforms/Scalar/Scalarizer.cpp
index 5cac4dca8cf8..e8fea501b1d8 100644
--- a/llvm/lib/Transforms/Scalar/Scalarizer.cpp
+++ b/llvm/lib/Transforms/Scalar/Scalarizer.cpp
@@ -22,8 +22,8 @@
#include "llvm/IR/BasicBlock.h"
#include "llvm/IR/Constants.h"
#include "llvm/IR/DataLayout.h"
-#include "llvm/IR/Dominators.h"
#include "llvm/IR/DerivedTypes.h"
+#include "llvm/IR/Dominators.h"
#include "llvm/IR/Function.h"
#include "llvm/IR/IRBuilder.h"
#include "llvm/IR/InstVisitor.h"
@@ -41,6 +41,7 @@
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/MathExtras.h"
#include "llvm/Transforms/Scalar.h"
+#include "llvm/Transforms/Utils/Local.h"
#include <cassert>
#include <cstdint>
#include <iterator>
@@ -222,6 +223,8 @@ class ScalarizerVisitor : public InstVisitor<ScalarizerVisitor, bool> {
ScatterMap Scattered;
GatherList Gathered;
+ SmallVector<WeakTrackingVH, 32> PotentiallyDeadInstrs;
+
unsigned ParallelLoopAccessMDKind;
DominatorTree *DT;
@@ -383,11 +386,6 @@ Scatterer ScalarizerVisitor::scatter(Instruction *Point, Value *V) {
// so that we can avoid creating the gathered form if all uses of Op are
// replaced with uses of CV.
void ScalarizerVisitor::gather(Instruction *Op, const ValueVector &CV) {
- // Since we're not deleting Op yet, stub out its operands, so that it
- // doesn't make anything live unnecessarily.
- for (unsigned I = 0, E = Op->getNumOperands(); I != E; ++I)
- Op->setOperand(I, UndefValue::get(Op->getOperand(I)->getType()));
-
transferMetadataAndIRFlags(Op, CV);
// If we already have a scattered form of Op (created from ExtractElements
@@ -402,7 +400,7 @@ void ScalarizerVisitor::gather(Instruction *Op, const ValueVector &CV) {
Instruction *Old = cast<Instruction>(V);
CV[I]->takeName(Old);
Old->replaceAllUsesWith(CV[I]);
- Old->eraseFromParent();
+ PotentiallyDeadInstrs.emplace_back(Old);
}
}
SV = CV;
@@ -950,10 +948,13 @@ bool ScalarizerVisitor::finish() {
Res->takeName(Op);
Op->replaceAllUsesWith(Res);
}
- Op->eraseFromParent();
+ PotentiallyDeadInstrs.emplace_back(Op);
}
Gathered.clear();
Scattered.clear();
+
+ RecursivelyDeleteTriviallyDeadInstructionsPermissive(PotentiallyDeadInstrs);
+
return true;
}
diff --git a/llvm/test/Transforms/Scalarizer/basic.ll b/llvm/test/Transforms/Scalarizer/basic.ll
index 2c7b6a6b588f..239cdd660a33 100644
--- a/llvm/test/Transforms/Scalarizer/basic.ll
+++ b/llvm/test/Transforms/Scalarizer/basic.ll
@@ -539,6 +539,19 @@ define <2 x float> @f22(<2 x float> %x, <2 x float> %y, <2 x float> %z) {
ret <2 x float> %res
}
+; See https://reviews.llvm.org/D83101#2133062
+define <2 x i32> @f23_crash(<2 x i32> %srcvec, i32 %v1) {
+; CHECK-LABEL: @f23_crash(
+; CHECK: %1 = extractelement <2 x i32> %srcvec, i32 0
+; CHECK: %t1.upto0 = insertelement <2 x i32> undef, i32 %1, i32 0
+; CHECK: %t1 = insertelement <2 x i32> %t1.upto0, i32 %v1, i32 1
+; CHECK: ret <2 x i32> %t1
+ %v0 = extractelement <2 x i32> %srcvec, i32 0
+ %t0 = insertelement <2 x i32> undef, i32 %v0, i32 0
+ %t1 = insertelement <2 x i32> %t0, i32 %v1, i32 1
+ ret <2 x i32> %t1
+}
+
!0 = !{ !"root" }
!1 = !{ !"set1", !0 }
!2 = !{ !"set2", !0 }
diff --git a/llvm/test/Transforms/Scalarizer/crash-bug.ll b/llvm/test/Transforms/Scalarizer/crash-bug.ll
index d0d019564977..d581707971e7 100644
--- a/llvm/test/Transforms/Scalarizer/crash-bug.ll
+++ b/llvm/test/Transforms/Scalarizer/crash-bug.ll
@@ -15,7 +15,6 @@ bb2: ; preds = %bb1
bb1: ; preds = %bb2, %0
%bb1_vec = phi <2 x i16> [ <i16 100, i16 200>, %0 ], [ %bb2_vec, %bb2 ]
;CHECK: bb1:
-;CHECK: %bb1_vec.i0 = phi i16 [ 100, %0 ], [ 0, %bb2 ]
;CHECK: %bb2_vec.i1 = phi i16 [ 200, %0 ], [ %bb2_vec.i1, %bb2 ]
br i1 undef, label %bb3, label %bb2
diff --git a/llvm/test/Transforms/Scalarizer/phi-unreachable-pred.ll b/llvm/test/Transforms/Scalarizer/phi-unreachable-pred.ll
index 8e89efb5d31f..9cfffe3b977f 100644
--- a/llvm/test/Transforms/Scalarizer/phi-unreachable-pred.ll
+++ b/llvm/test/Transforms/Scalarizer/phi-unreachable-pred.ll
@@ -11,11 +11,8 @@ define i16 @f1() {
; CHECK: for.cond:
; CHECK-NEXT: br i1 undef, label [[FOR_BODY:%.*]], label [[FOR_END]]
; CHECK: for.end:
-; CHECK-NEXT: [[PHI_I0:%.*]] = phi i16 [ 1, [[ENTRY:%.*]] ], [ undef, [[FOR_COND]] ]
-; CHECK-NEXT: [[PHI_I1:%.*]] = phi i16 [ 1, [[ENTRY]] ], [ undef, [[FOR_COND]] ]
-; CHECK-NEXT: [[PHI_I2:%.*]] = phi i16 [ 1, [[ENTRY]] ], [ undef, [[FOR_COND]] ]
-; CHECK-NEXT: [[PHI_I3:%.*]] = phi i16 [ 1, [[ENTRY]] ], [ undef, [[FOR_COND]] ]
-; CHECK-NEXT: ret i16 [[PHI_I0]]
+; CHECK-NEXT: [[EXTRACT:%.*]] = phi i16 [ 1, [[ENTRY:%.*]] ], [ undef, [[FOR_COND]] ]
+; CHECK-NEXT: ret i16 [[EXTRACT]]
;
entry:
br label %for.end
More information about the llvm-commits
mailing list