From a22205806d257629697325c69d8a166539ddc5e3 Mon Sep 17 00:00:00 2001 From: Bjorn Pettersson Date: Wed, 12 Apr 2017 08:07:55 +0000 Subject: [PATCH] [LoadCombine] Avoid analysing dead basic blocks Summary: Dead basic blocks may be forming a loop, for which SSA form is fulfilled, but with a circular def-use chain. LoadCombine could enter an infinite loop when analysing such dead code. This patch solves the problem by simply avoiding to analyse all basic blocks that aren't forward reachable, from function entry, in LoadCombine. Fixes https://bugs.llvm.org/show_bug.cgi?id=27065 Reviewers: mehdi_amini, chandlerc, grosser, Bigcheese, davide Reviewed By: davide Subscribers: dberlin, zzheng, bjope, grandinj, Ka-Ka, materi, jholewinski, llvm-commits, mzolotukhin Differential Revision: https://reviews.llvm.org/D31032 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@300034 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Transforms/Scalar/LoadCombine.cpp | 13 ++++++++- test/Transforms/LoadCombine/deadcode.ll | 39 +++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 test/Transforms/LoadCombine/deadcode.ll diff --git a/lib/Transforms/Scalar/LoadCombine.cpp b/lib/Transforms/Scalar/LoadCombine.cpp index 61b7804b59b..02215d3450c 100644 --- a/lib/Transforms/Scalar/LoadCombine.cpp +++ b/lib/Transforms/Scalar/LoadCombine.cpp @@ -19,6 +19,7 @@ #include "llvm/Analysis/GlobalsModRef.h" #include "llvm/Analysis/TargetFolder.h" #include "llvm/IR/DataLayout.h" +#include "llvm/IR/Dominators.h" #include "llvm/IR/Function.h" #include "llvm/IR/IRBuilder.h" #include "llvm/IR/Instructions.h" @@ -53,18 +54,20 @@ struct LoadPOPPair { class LoadCombine : public BasicBlockPass { LLVMContext *C; AliasAnalysis *AA; + DominatorTree *DT; public: LoadCombine() : BasicBlockPass(ID), C(nullptr), AA(nullptr) { initializeLoadCombinePass(*PassRegistry::getPassRegistry()); } - + using llvm::Pass::doInitialization; bool doInitialization(Function &) override; bool runOnBasicBlock(BasicBlock &BB) override; void getAnalysisUsage(AnalysisUsage &AU) const override { AU.setPreservesCFG(); AU.addRequired(); + AU.addRequired(); AU.addPreserved(); } @@ -234,6 +237,14 @@ bool LoadCombine::runOnBasicBlock(BasicBlock &BB) { return false; AA = &getAnalysis().getAAResults(); + DT = &getAnalysis().getDomTree(); + + // Skip analysing dead blocks (not forward reachable from function entry). + if (!DT->isReachableFromEntry(&BB)) { + DEBUG(dbgs() << "LC: skipping unreachable " << BB.getName() << + " in " << BB.getParent()->getName() << "\n"); + return false; + } IRBuilder TheBuilder( BB.getContext(), TargetFolder(BB.getModule()->getDataLayout())); diff --git a/test/Transforms/LoadCombine/deadcode.ll b/test/Transforms/LoadCombine/deadcode.ll new file mode 100644 index 00000000000..ed72824ffb4 --- /dev/null +++ b/test/Transforms/LoadCombine/deadcode.ll @@ -0,0 +1,39 @@ +; NOTE: Assertions have been autogenerated by utils/update_test_checks.py +; RUN: opt -load-combine -S < %s | FileCheck %s + +; It has been detected that dead loops like the one in this test case can be +; created by -jump-threading (it was detected by a csmith generated program). +; +; According to -verify this is valid input (even if it could be discussed if +; the dead loop really satisfies SSA form). +; +; The problem found was that the -load-combine pass ends up in an infinite loop +; when analysing the 'bb1' basic block. +define void @test1() { +; CHECK-LABEL: @test1( +; CHECK-NEXT: ret void +; CHECK: bb1: +; CHECK-NEXT: [[_TMP4:%.*]] = load i16, i16* [[_TMP10:%.*]], align 1 +; CHECK-NEXT: [[_TMP10]] = getelementptr i16, i16* [[_TMP10]], i16 1 +; CHECK-NEXT: br label [[BB1:%.*]] +; CHECK: bb2: +; CHECK-NEXT: [[_TMP7:%.*]] = load i16, i16* [[_TMP12:%.*]], align 1 +; CHECK-NEXT: [[_TMP12]] = getelementptr i16, i16* [[_TMP12]], i16 1 +; CHECK-NEXT: br label [[BB2:%.*]] +; + ret void + +bb1: + %_tmp4 = load i16, i16* %_tmp10, align 1 + %_tmp10 = getelementptr i16, i16* %_tmp10, i16 1 + br label %bb1 + +; A second basic block. Running the test with -debug-pass=Executions shows +; that we only run the Dominator Tree Construction one time for each function, +; also when having multiple basic blocks in the function. +bb2: + %_tmp7 = load i16, i16* %_tmp12, align 1 + %_tmp12 = getelementptr i16, i16* %_tmp12, i16 1 + br label %bb2 + +}