[llvm] r347979 - [LoopSimplifyCFG] Update MemorySSA in terminator folding. PR39783
Maxim Kazantsev via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 6 01:53:07 PST 2018
Fix on review: https://reviews.llvm.org/D55357
Thinking a bit more about this, we can support irreducible CFG if we change the algorithms, but since this CFG is rare, we can do it later.
--Max
-----Original Message-----
From: Maxim Kazantsev
Sent: Thursday, December 6, 2018 3:36 PM
To: 'Mikael Holmén' <mikael.holmen at ericsson.com>
Cc: llvm-commits at lists.llvm.org
Subject: RE: [llvm] r347979 - [LoopSimplifyCFG] Update MemorySSA in terminator folding. PR39783
Hi Mikael,
Thanks for the test. After taking a look into this problem, it seems that the collection algorithm breaks in presence of irreducible CFG in the current loop, because in this case RPO actually makes no sense. I will try to find out how to fix it.
--Max
-----Original Message-----
From: Mikael Holmén <mikael.holmen at ericsson.com>
Sent: Thursday, December 6, 2018 2:01 PM
To: Maxim Kazantsev <max.kazantsev at azul.com>
Cc: llvm-commits at lists.llvm.org
Subject: Re: [llvm] r347979 - [LoopSimplifyCFG] Update MemorySSA in terminator folding. PR39783
Hi Max,
With the term-folding enabled again, the following crashes for me:
opt -S -loop-simplifycfg -o - bbi-21921.ll
opt: ../lib/Transforms/Scalar/LoopSimplifyCFG.cpp:188: void
ConstantTerminatorFoldingImpl::analyze(): Assertion `L.getNumBlocks() ==
LiveLoopBlocks.size() + DeadLoopBlocks.size() && "Malformed block sets?"' failed.
Stack dump:
0. Program arguments: build-all/bin/opt -S -loop-simplifycfg -o -
bbi-21921.ll
1. Running pass 'Function Pass Manager' on module 'bbi-21921.ll'.
2. Running pass 'Loop Pass Manager' on function '@f'
3. Running pass 'Simplify loop CFG' on basic block '%lbl1'
#0 0x0000000002226344 PrintStackTraceSignalHandler(void*)
(build-all/bin/opt+0x2226344)
#1 0x0000000002224470 llvm::sys::RunSignalHandlers()
(build-all/bin/opt+0x2224470)
#2 0x00000000022266a8 SignalHandler(int) (build-all/bin/opt+0x22266a8)
#3 0x00007f6392a34330 __restore_rt
(/lib/x86_64-linux-gnu/libpthread.so.0+0x10330)
#4 0x00007f6391623c37 gsignal
/build/eglibc-ripdx6/eglibc-2.19/signal/../nptl/sysdeps/unix/sysv/linux/raise.c:56:0
#5 0x00007f6391627028 abort
/build/eglibc-ripdx6/eglibc-2.19/stdlib/abort.c:91:0
#6 0x00007f639161cbf6 __assert_fail_base
/build/eglibc-ripdx6/eglibc-2.19/assert/assert.c:92:0
#7 0x00007f639161cca2 (/lib/x86_64-linux-gnu/libc.so.6+0x2fca2)
#8 0x000000000206d6ce ConstantTerminatorFoldingImpl::analyze()
(build-all/bin/opt+0x206d6ce)
#9 0x000000000206c1b4 ConstantTerminatorFoldingImpl::run()
(build-all/bin/opt+0x206c1b4)
#10 0x000000000206b3d5 simplifyLoopCFG(llvm::Loop&, llvm::DominatorTree&, llvm::LoopInfo&, llvm::ScalarEvolution&,
llvm::MemorySSAUpdater*) (build-all/bin/opt+0x206b3d5)
#11 0x000000000206beb3 (anonymous
namespace)::LoopSimplifyCFGLegacyPass::runOnLoop(llvm::Loop*,
llvm::LPPassManager&) (build-all/bin/opt+0x206beb3)
#12 0x000000000169db1c
llvm::LPPassManager::runOnFunction(llvm::Function&)
(build-all/bin/opt+0x169db1c)
#13 0x0000000001c3cfbd
llvm::FPPassManager::runOnFunction(llvm::Function&)
(build-all/bin/opt+0x1c3cfbd)
#14 0x0000000001c3d278 llvm::FPPassManager::runOnModule(llvm::Module&)
(build-all/bin/opt+0x1c3d278)
#15 0x0000000001c3d6da llvm::legacy::PassManagerImpl::run(llvm::Module&)
(build-all/bin/opt+0x1c3d6da)
#16 0x000000000078119b main (build-all/bin/opt+0x78119b)
#17 0x00007f639160ef45 __libc_start_main
/build/eglibc-ripdx6/eglibc-2.19/csu/libc-start.c:321:0
#18 0x00000000007667fd _start (build-all/bin/opt+0x7667fd) Abort
I tried on top-of-tree now and it still crashes.
/Mikael
On 11/30/18 11:06 AM, Max Kazantsev via llvm-commits wrote:
> Author: mkazantsev
> Date: Fri Nov 30 02:06:23 2018
> New Revision: 347979
>
> URL: http://llvm.org/viewvc/llvm-project?rev=347979&view=rev
> Log:
> [LoopSimplifyCFG] Update MemorySSA in terminator folding. PR39783
>
> Terminator folding transform lacks MemorySSA update for memory Phis,
> while they exist within MemorySSA analysis. They need exactly the same
> type of updates as regular Phis. Failing to update them properly ends
> up with inconsistent MemorySSA and manifests in various assertion failures.
>
> This patch adds Memory Phi updates to this transform.
>
> Thanks to @jonpa for finding this!
>
> Differential Revision: https://reviews.llvm.org/D55050 Reviewed By:
> asbirlea
>
> Modified:
> llvm/trunk/lib/Transforms/Scalar/LoopSimplifyCFG.cpp
> llvm/trunk/test/Transforms/LoopSimplifyCFG/pr39783.ll
>
> Modified: llvm/trunk/lib/Transforms/Scalar/LoopSimplifyCFG.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/L
> oopSimplifyCFG.cpp?rev=347979&r1=347978&r2=347979&view=diff
> ======================================================================
> ========
> --- llvm/trunk/lib/Transforms/Scalar/LoopSimplifyCFG.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/LoopSimplifyCFG.cpp Fri Nov 30
> +++ 02:06:23 2018
> @@ -42,7 +42,7 @@ using namespace llvm;
> #define DEBUG_TYPE "loop-simplifycfg"
>
> static cl::opt<bool> EnableTermFolding("enable-loop-simplifycfg-term-folding",
> - cl::init(false));
> + cl::init(true));
>
> STATISTIC(NumTerminatorsFolded,
> "Number of terminators folded to unconditional branches");
> @@ -83,6 +83,7 @@ private:
> Loop &L;
> LoopInfo &LI;
> DominatorTree &DT;
> + MemorySSAUpdater *MSSAU;
>
> // Whether or not the current loop will still exist after terminator constant
> // folding will be done. In theory, there are two ways how it can happen:
> @@ -257,6 +258,8 @@ private:
> // the one-input Phi because it is a LCSSA Phi.
> bool PreserveLCSSAPhi = !L.contains(Succ);
> Succ->removePredecessor(BB, PreserveLCSSAPhi);
> + if (MSSAU)
> + MSSAU->removeEdge(BB, Succ);
> } else
> ++TheOnlySuccDuplicates;
>
> @@ -267,6 +270,8 @@ private:
> bool PreserveLCSSAPhi = !L.contains(TheOnlySucc);
> for (unsigned Dup = 1; Dup < TheOnlySuccDuplicates; ++Dup)
> TheOnlySucc->removePredecessor(BB, PreserveLCSSAPhi);
> + if (MSSAU && TheOnlySuccDuplicates > 1)
> + MSSAU->removeDuplicatePhiEdgesBetween(BB, TheOnlySucc);
>
> IRBuilder<> Builder(BB->getContext());
> Instruction *Term = BB->getTerminator(); @@ -282,8 +287,9 @@
> private:
> }
>
> public:
> - ConstantTerminatorFoldingImpl(Loop &L, LoopInfo &LI, DominatorTree &DT)
> - : L(L), LI(LI), DT(DT) {}
> + ConstantTerminatorFoldingImpl(Loop &L, LoopInfo &LI, DominatorTree &DT,
> + MemorySSAUpdater *MSSAU)
> + : L(L), LI(LI), DT(DT), MSSAU(MSSAU) {}
> bool run() {
> assert(L.getLoopLatch() && "Should be single latch!");
>
> @@ -364,7 +370,8 @@ public:
>
> /// Turn branches and switches with known constant conditions into unconditional
> /// branches.
> -static bool constantFoldTerminators(Loop &L, DominatorTree &DT,
> LoopInfo &LI) {
> +static bool constantFoldTerminators(Loop &L, DominatorTree &DT, LoopInfo &LI,
> + MemorySSAUpdater *MSSAU) {
> if (!EnableTermFolding)
> return false;
>
> @@ -373,7 +380,7 @@ static bool constantFoldTerminators(Loop
> if (!L.getLoopLatch())
> return false;
>
> - ConstantTerminatorFoldingImpl BranchFolder(L, LI, DT);
> + ConstantTerminatorFoldingImpl BranchFolder(L, LI, DT, MSSAU);
> return BranchFolder.run();
> }
>
> @@ -410,7 +417,7 @@ static bool simplifyLoopCFG(Loop &L, Dom
> bool Changed = false;
>
> // Constant-fold terminators with known constant conditions.
> - Changed |= constantFoldTerminators(L, DT, LI);
> + Changed |= constantFoldTerminators(L, DT, LI, MSSAU);
>
> // Eliminate unconditional branches by merging blocks into their predecessors.
> Changed |= mergeBlocksIntoPredecessors(L, DT, LI, MSSAU);
>
> Modified: llvm/trunk/test/Transforms/LoopSimplifyCFG/pr39783.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopSim
> plifyCFG/pr39783.ll?rev=347979&r1=347978&r2=347979&view=diff
> ======================================================================
> ========
> --- llvm/trunk/test/Transforms/LoopSimplifyCFG/pr39783.ll (original)
> +++ llvm/trunk/test/Transforms/LoopSimplifyCFG/pr39783.ll Fri Nov 30
> +++ 02:06:23 2018
> @@ -1,4 +1,3 @@
> -; XFAIL: *
> ; REQUIRES: asserts
> ; RUN: opt -march=z13 -S -loop-simplifycfg -enable-mssa-loop-dependency -enable-loop-simplifycfg-term-folding 2>&1 < %s | FileCheck %s
> target datalayout = "E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-v128:64-a:8:16-n32:64"
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
More information about the llvm-commits
mailing list