[llvm] r330403 - Reapply "[PR16756] Use SSAUpdaterBulk in JumpThreading." one more time.
Ilya Biryukov via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 20 03:51:16 PDT 2018
+Hans Wennborg <hwennborg at google.com> , who can provide details for a repro
on top of Chromium codebase.
On Fri, Apr 20, 2018 at 12:48 PM Michael Zolotukhin <mzolotukhin at apple.com>
wrote:
>
>
> On Apr 20, 2018, at 2:43 PM, Ilya Biryukov <ibiryukov at google.com> wrote:
>
> This commit seems to crash clang during our integrate while doing PGO
> build with the following crash:
> #2 llvm::SSAUpdaterBulk::RewriteAllUses(llvm::DominatorTree*,
> llvm::SmallVectorImpl<llvm::PHINode*>*)
> #3 llvm::JumpThreadingPass::ThreadEdge(llvm::BasicBlock*,
> llvm::SmallVectorImpl<llvm::BasicBlock*> const&, llvm::BasicBlock*)
> #4 llvm::JumpThreadingPass::ProcessThreadableEdges(llvm::Value*,
> llvm::BasicBlock*, llvm::jumpthreading::ConstantPreference,
> llvm::Instruction*)
> #5 llvm::JumpThreadingPass::ProcessBlock(llvm::BasicBlock*)
>
> The crash happens while compiling 'lib/Analysis/CallGraph.cpp'.
>
> I'm planning to revert it, any objections?
>
> No objections, but could you please send me a reproducer please? So far
> the only testing this change didn’t pass was stage3/stage4 compiler compare
> (binaries are different with my change), but this isn’t very helpful when
> you don’t know where the bug is.
>
> Michael
>
>
> On Fri, Apr 20, 2018 at 10:04 AM Michael Zolotukhin via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: mzolotukhin
>> Date: Fri Apr 20 01:01:08 2018
>> New Revision: 330403
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=330403&view=rev
>> Log:
>> Reapply "[PR16756] Use SSAUpdaterBulk in JumpThreading." one more time.
>>
>> Hopefully, changing set to vector removes nondeterminism detected by
>> some bots, or the new assert will catch something.
>>
>> This reverts commit r330180.
>>
>> Added:
>> llvm/trunk/test/Transforms/JumpThreading/removed-use.ll
>> Modified:
>> llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp
>>
>> Modified: llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp?rev=330403&r1=330402&r2=330403&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp Fri Apr 20
>> 01:01:08 2018
>> @@ -66,6 +66,7 @@
>> #include "llvm/Transforms/Utils/BasicBlockUtils.h"
>> #include "llvm/Transforms/Utils/Cloning.h"
>> #include "llvm/Transforms/Utils/SSAUpdater.h"
>> +#include "llvm/Transforms/Utils/SSAUpdaterBulk.h"
>> #include "llvm/Transforms/Utils/ValueMapper.h"
>> #include <algorithm>
>> #include <cassert>
>> @@ -1989,15 +1990,20 @@ bool JumpThreadingPass::ThreadEdge(Basic
>> // now have to update all uses of the value to use either the original
>> value,
>> // the cloned value, or some PHI derived value. This can require
>> arbitrary
>> // PHI insertion, of which we are prepared to do, clean these up now.
>> - SSAUpdater SSAUpdate;
>> - SmallVector<Use*, 16> UsesToRename;
>> + SSAUpdaterBulk SSAUpdate;
>> +
>> + unsigned VarNum = 0;
>> for (Instruction &I : *BB) {
>> + SmallVector<Use*, 16> UsesToRename;
>> +
>> // Scan all uses of this instruction to see if it is used outside of
>> its
>> - // block, and if so, record them in UsesToRename.
>> + // block, and if so, record them in UsesToRename. Also, skip phi
>> operands
>> + // from PredBB - we'll remove them anyway.
>> for (Use &U : I.uses()) {
>> Instruction *User = cast<Instruction>(U.getUser());
>> if (PHINode *UserPN = dyn_cast<PHINode>(User)) {
>> - if (UserPN->getIncomingBlock(U) == BB)
>> + if (UserPN->getIncomingBlock(U) == BB ||
>> + UserPN->getIncomingBlock(U) == PredBB)
>> continue;
>> } else if (User->getParent() == BB)
>> continue;
>> @@ -2008,19 +2014,15 @@ bool JumpThreadingPass::ThreadEdge(Basic
>> // If there are no uses outside the block, we're done with this
>> instruction.
>> if (UsesToRename.empty())
>> continue;
>> + SSAUpdate.AddVariable(VarNum, I.getName(), I.getType());
>>
>> - DEBUG(dbgs() << "JT: Renaming non-local uses of: " << I << "\n");
>> -
>> - // We found a use of I outside of BB. Rename all uses of I that are
>> outside
>> - // its block to be uses of the appropriate PHI node etc. See
>> ValuesInBlocks
>> - // with the two values we know.
>> - SSAUpdate.Initialize(I.getType(), I.getName());
>> - SSAUpdate.AddAvailableValue(BB, &I);
>> - SSAUpdate.AddAvailableValue(NewBB, ValueMapping[&I]);
>> -
>> - while (!UsesToRename.empty())
>> - SSAUpdate.RewriteUse(*UsesToRename.pop_back_val());
>> - DEBUG(dbgs() << "\n");
>> + // We found a use of I outside of BB - we need to rename all uses of
>> I that
>> + // are outside its block to be uses of the appropriate PHI node etc.
>> + SSAUpdate.AddAvailableValue(VarNum, BB, &I);
>> + SSAUpdate.AddAvailableValue(VarNum, NewBB, ValueMapping[&I]);
>> + for (auto *U : UsesToRename)
>> + SSAUpdate.AddUse(VarNum, U);
>> + VarNum++;
>> }
>>
>> // Ok, NewBB is good to go. Update the terminator of PredBB to jump to
>> @@ -2037,6 +2039,10 @@ bool JumpThreadingPass::ThreadEdge(Basic
>> {DominatorTree::Insert, PredBB, NewBB},
>> {DominatorTree::Delete, PredBB, BB}});
>>
>> + // Apply all updates we queued with DDT and get the updated Dominator
>> Tree.
>> + DominatorTree *DT = &DDT->flush();
>> + SSAUpdate.RewriteAllUses(DT);
>> +
>> // At this point, the IR is fully up to date and consistent. Do a
>> quick scan
>> // over the new instructions and zap any that are constants or dead.
>> This
>> // frequently happens because of phi translation.
>>
>> Added: llvm/trunk/test/Transforms/JumpThreading/removed-use.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/JumpThreading/removed-use.ll?rev=330403&view=auto
>>
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/JumpThreading/removed-use.ll (added)
>> +++ llvm/trunk/test/Transforms/JumpThreading/removed-use.ll Fri Apr 20
>> 01:01:08 2018
>> @@ -0,0 +1,28 @@
>> +; RUN: opt -S < %s -jump-threading | FileCheck %s
>> +; CHECK-LABEL: @foo
>> +; CHECK: bb6:
>> +; CHECK-NEXT: ret void
>> +; CHECK: bb3:
>> +; CHECK: br label %bb3
>> +define void @foo() {
>> +entry:
>> + br i1 true, label %bb6, label %bb3
>> +
>> +bb3:
>> + %x0 = phi i32 [ undef, %entry ], [ %x1, %bb5 ]
>> + %y = and i64 undef, 1
>> + %p = icmp ne i64 %y, 0
>> + br i1 %p, label %bb4, label %bb5
>> +
>> +bb4:
>> + br label %bb5
>> +
>> +bb5:
>> + %x1 = phi i32 [ %x0, %bb3 ], [ %x0, %bb4 ]
>> + %z = phi i32 [ 0, %bb3 ], [ 1, %bb4 ]
>> + %q = icmp eq i32 %z, 0
>> + br i1 %q, label %bb3, label %bb6
>> +
>> +bb6:
>> + ret void
>> +}
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
>
> --
> Regards,
> Ilya Biryukov
>
>
>
--
Regards,
Ilya Biryukov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180420/40bd5379/attachment.html>
More information about the llvm-commits
mailing list