[llvm] r330403 - Reapply "[PR16756] Use SSAUpdaterBulk in JumpThreading." one more time.
Michael Zolotukhin via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 24 06:43:14 PDT 2018
Hi Hans,
It’s just a gentle ping: could you please check if your build succeeded or failed with r330434? Your previous reproducer was of a great value to me and if your build exposes more bugs, I’d really appreciate if you could help me finding reproducers again.
Thanks,
Michael
> On Apr 20, 2018, at 6:56 PM, Michael Zolotukhin <mzolotukhin at apple.com> wrote:
>
> Hi Hans,
>
> Does your testing pass with r330434? I fixed the bug from the reproducer you sent to me, but I still see stage3/stage4 miscompares (http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu/builds/9858). Im going to revert the patch (again :-( ), but was wondering if your testing would help me with another reproducer.
>
> Thanks,
> Michael
>
>> On Apr 20, 2018, at 3:09 PM, Michael Zolotukhin via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>
>> Thanks, the reproducer works for me! And sorry for the breakage.
>>
>> Michael
>>
>>> On Apr 20, 2018, at 3:00 PM, Hans Wennborg <hans at chromium.org> wrote:
>>>
>>> Uploaded the repro here:
>>> https://bugs.chromium.org/p/chromium/issues/detail?id=835245#c1
>>> Please let me know if there are any issues with reproducing.
>>>
>>> On Fri, Apr 20, 2018 at 11:51 AM, Ilya Biryukov <ibiryukov at google.com> wrote:
>>>> +Hans Wennborg , 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
>>
>> _______________________________________________
>> 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