<div dir="ltr"><a class="gmail_plusreply" id="plusReplyChip-0" href="mailto:hwennborg@google.com" tabindex="-1">+Hans Wennborg</a> , who can provide details for a repro on top of Chromium codebase.</div><br><div class="gmail_quote"><div dir="ltr">On Fri, Apr 20, 2018 at 12:48 PM Michael Zolotukhin <<a href="mailto:mzolotukhin@apple.com">mzolotukhin@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space"><br><div><br><blockquote type="cite"><div>On Apr 20, 2018, at 2:43 PM, Ilya Biryukov <<a href="mailto:ibiryukov@google.com" target="_blank">ibiryukov@google.com</a>> wrote:</div><br class="m_-7213928487619890922Apple-interchange-newline"><div><div dir="ltr">This commit seems to crash clang during our integrate while doing PGO build with the following crash:<div><div> #2 llvm::SSAUpdaterBulk::RewriteAllUses(llvm::DominatorTree*, llvm::SmallVectorImpl<llvm::PHINode*>*)<br></div><div> #3 llvm::JumpThreadingPass::ThreadEdge(llvm::BasicBlock*, llvm::SmallVectorImpl<llvm::BasicBlock*> const&, llvm::BasicBlock*)</div><div> #4 llvm::JumpThreadingPass::ProcessThreadableEdges(llvm::Value*, llvm::BasicBlock*, llvm::jumpthreading::ConstantPreference, llvm::Instruction*)</div><div> #5 llvm::JumpThreadingPass::ProcessBlock(llvm::BasicBlock*)</div><div> </div><div>The crash happens while compiling 'lib/Analysis/CallGraph.cpp'.</div></div><div><br></div><div>I'm planning to revert it, any objections?</div></div></div></blockquote>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.</div><div><br></div><div>Michael<br><blockquote type="cite"><div><br><div class="gmail_quote"><div dir="ltr">On Fri, Apr 20, 2018 at 10:04 AM Michael Zolotukhin via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: mzolotukhin<br>
Date: Fri Apr 20 01:01:08 2018<br>
New Revision: 330403<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=330403&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=330403&view=rev</a><br>
Log:<br>
Reapply "[PR16756] Use SSAUpdaterBulk in JumpThreading." one more time.<br>
<br>
Hopefully, changing set to vector removes nondeterminism detected by<br>
some bots, or the new assert will catch something.<br>
<br>
This reverts commit r330180.<br>
<br>
Added:<br>
llvm/trunk/test/Transforms/JumpThreading/removed-use.ll<br>
Modified:<br>
llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp<br>
<br>
Modified: llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp?rev=330403&r1=330402&r2=330403&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp?rev=330403&r1=330402&r2=330403&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp Fri Apr 20 01:01:08 2018<br>
@@ -66,6 +66,7 @@<br>
#include "llvm/Transforms/Utils/BasicBlockUtils.h"<br>
#include "llvm/Transforms/Utils/Cloning.h"<br>
#include "llvm/Transforms/Utils/SSAUpdater.h"<br>
+#include "llvm/Transforms/Utils/SSAUpdaterBulk.h"<br>
#include "llvm/Transforms/Utils/ValueMapper.h"<br>
#include <algorithm><br>
#include <cassert><br>
@@ -1989,15 +1990,20 @@ bool JumpThreadingPass::ThreadEdge(Basic<br>
// now have to update all uses of the value to use either the original value,<br>
// the cloned value, or some PHI derived value. This can require arbitrary<br>
// PHI insertion, of which we are prepared to do, clean these up now.<br>
- SSAUpdater SSAUpdate;<br>
- SmallVector<Use*, 16> UsesToRename;<br>
+ SSAUpdaterBulk SSAUpdate;<br>
+<br>
+ unsigned VarNum = 0;<br>
for (Instruction &I : *BB) {<br>
+ SmallVector<Use*, 16> UsesToRename;<br>
+<br>
// Scan all uses of this instruction to see if it is used outside of its<br>
- // block, and if so, record them in UsesToRename.<br>
+ // block, and if so, record them in UsesToRename. Also, skip phi operands<br>
+ // from PredBB - we'll remove them anyway.<br>
for (Use &U : I.uses()) {<br>
Instruction *User = cast<Instruction>(U.getUser());<br>
if (PHINode *UserPN = dyn_cast<PHINode>(User)) {<br>
- if (UserPN->getIncomingBlock(U) == BB)<br>
+ if (UserPN->getIncomingBlock(U) == BB ||<br>
+ UserPN->getIncomingBlock(U) == PredBB)<br>
continue;<br>
} else if (User->getParent() == BB)<br>
continue;<br>
@@ -2008,19 +2014,15 @@ bool JumpThreadingPass::ThreadEdge(Basic<br>
// If there are no uses outside the block, we're done with this instruction.<br>
if (UsesToRename.empty())<br>
continue;<br>
+ SSAUpdate.AddVariable(VarNum, I.getName(), I.getType());<br>
<br>
- DEBUG(dbgs() << "JT: Renaming non-local uses of: " << I << "\n");<br>
-<br>
- // We found a use of I outside of BB. Rename all uses of I that are outside<br>
- // its block to be uses of the appropriate PHI node etc. See ValuesInBlocks<br>
- // with the two values we know.<br>
- SSAUpdate.Initialize(I.getType(), I.getName());<br>
- SSAUpdate.AddAvailableValue(BB, &I);<br>
- SSAUpdate.AddAvailableValue(NewBB, ValueMapping[&I]);<br>
-<br>
- while (!UsesToRename.empty())<br>
- SSAUpdate.RewriteUse(*UsesToRename.pop_back_val());<br>
- DEBUG(dbgs() << "\n");<br>
+ // We found a use of I outside of BB - we need to rename all uses of I that<br>
+ // are outside its block to be uses of the appropriate PHI node etc.<br>
+ SSAUpdate.AddAvailableValue(VarNum, BB, &I);<br>
+ SSAUpdate.AddAvailableValue(VarNum, NewBB, ValueMapping[&I]);<br>
+ for (auto *U : UsesToRename)<br>
+ SSAUpdate.AddUse(VarNum, U);<br>
+ VarNum++;<br>
}<br>
<br>
// Ok, NewBB is good to go. Update the terminator of PredBB to jump to<br>
@@ -2037,6 +2039,10 @@ bool JumpThreadingPass::ThreadEdge(Basic<br>
{DominatorTree::Insert, PredBB, NewBB},<br>
{DominatorTree::Delete, PredBB, BB}});<br>
<br>
+ // Apply all updates we queued with DDT and get the updated Dominator Tree.<br>
+ DominatorTree *DT = &DDT->flush();<br>
+ SSAUpdate.RewriteAllUses(DT);<br>
+<br>
// At this point, the IR is fully up to date and consistent. Do a quick scan<br>
// over the new instructions and zap any that are constants or dead. This<br>
// frequently happens because of phi translation.<br>
<br>
Added: llvm/trunk/test/Transforms/JumpThreading/removed-use.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/JumpThreading/removed-use.ll?rev=330403&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/JumpThreading/removed-use.ll?rev=330403&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/JumpThreading/removed-use.ll (added)<br>
+++ llvm/trunk/test/Transforms/JumpThreading/removed-use.ll Fri Apr 20 01:01:08 2018<br>
@@ -0,0 +1,28 @@<br>
+; RUN: opt -S < %s -jump-threading | FileCheck %s<br>
+; CHECK-LABEL: @foo<br>
+; CHECK: bb6:<br>
+; CHECK-NEXT: ret void<br>
+; CHECK: bb3:<br>
+; CHECK: br label %bb3<br>
+define void @foo() {<br>
+entry:<br>
+ br i1 true, label %bb6, label %bb3<br>
+<br>
+bb3:<br>
+ %x0 = phi i32 [ undef, %entry ], [ %x1, %bb5 ]<br>
+ %y = and i64 undef, 1<br>
+ %p = icmp ne i64 %y, 0<br>
+ br i1 %p, label %bb4, label %bb5<br>
+<br>
+bb4:<br>
+ br label %bb5<br>
+<br>
+bb5:<br>
+ %x1 = phi i32 [ %x0, %bb3 ], [ %x0, %bb4 ]<br>
+ %z = phi i32 [ 0, %bb3 ], [ 1, %bb4 ]<br>
+ %q = icmp eq i32 %z, 0<br>
+ br i1 %q, label %bb3, label %bb6<br>
+<br>
+bb6:<br>
+ ret void<br>
+}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="m_-7213928487619890922gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div>Regards,</div><div>Ilya Biryukov</div></div></div></div></div>
</div></blockquote></div><br></div></blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div>Regards,</div><div>Ilya Biryukov</div></div></div></div></div>