- User Since
- Jul 4 2017, 1:39 AM (167 w, 5 d)
Sun, Sep 13
Fri, Sep 11
RecursivelyDeleteTriviallyDeadInstructions() call is wrapped with resetIteratorIfInvalidatedWhileCalling().
Ok, but it will be less efficient.
Thu, Sep 10
Addressed Fedor's comments.
Wed, Sep 9
Fedor, could you please review the New Pass Manager part of this patch as I addressed your comments?
Fri, Sep 4
I see no impact on CPU time with the following timed runs (both with and without assertions):
$ time opt -O3 FILE.bc --disable-output -verify-cfg-preserved=VCP
Thu, Sep 3
a minor fix in BBGuard.
Implemented custom BBGuard class instead of PoisoningVH which does not track poisoning in release builds.
Removed DEBUG_TYPE and LLVM_DEBUG uses. Unexpected gaph diffs are printed unconditionally.
I will provide performance data soon.
I will collect performance data and fix for release builds.
Wed, Sep 2
No further complains from me.
lgtm if the other reviewers' comments are addressed.
Tue, Sep 1
Defined a separate class named CFG.
CFG defines the notion PreserveCFG for the multigraph of BasicBlocks with unordered successors.
CFG can safely track and print CFG changes. To guard CFG from accessing deleted blocks PoisoningVH is used and is extended with one getter.
The cfg diff printing function is rewritten.
Sun, Aug 30
Thu, Aug 27
Wed, Aug 26
Sun, Aug 23
Aug 21 2020
Aug 20 2020
Aug 13 2020
Aug 6 2020
ok. As I see, both failures have been fixed. Thanks @Meinersbur for fixing polly/test/Isl/Ast/alias_checks_with_empty_context.ll
Aug 5 2020
Aug 4 2020
Thanks. I should have tested some more than just llvm.
Aug 3 2020
Reverted NodeOrderMap from SmallDenseMap to DenseMap.
changed type of SuccOrder to SmallDenseMap<NodePtr, unsigned, 16>.
Aug 2 2020
I have not checked compile time impact. I see 3 options:
added comment on SuccOrder parameter of runDFS().
Jul 30 2020
rebased + addressed comments (minor)
Added a test case with two reverse unreachable subgraphs.
Jul 29 2020
From my point of view I'm just fixing the bug 46098.
I suggest that we work on this change with a normal priority unless the fix is needed in 11.0.
Jul 28 2020
Removed the class Instrument and moved the class declaration of PreservedCFGCheckerInstrumentation from cpp to h file.
Relaxed CFG check for successors order (to allow branch successors swap).
Made CFG diff printing safe for possible block removal.
Fixed push-pop CFG to/from stack and a parallel stack with pass names for assertions.
I'm about to update the patches.
Jul 24 2020
I have just proposed a solution alternative to D81089. The intention is clear: do CFG changes in SimplifyCFG instead of InstCombine. Strictly speaking PreserveCFG must not be reported if we change terminators in any way.
Thank you for pointing this out. I will try changing canonicalizeICmpPredicate(), canFreelyInvertAllUsersOf().
Jul 23 2020
Jul 22 2020
Jul 21 2020
Jun 19 2020
Jun 18 2020
Landed as 707836ed4edb21e7133007f0200c3fd3a04d3409
Jun 17 2020
Thank you for reporting the case.
The bug is fixed with https://reviews.llvm.org/D81996 by Hans Wennborg.
Jun 11 2020
I do not like that the current design makes me expose of a new standard instrumentation class declaration for all users of StandardInstrumentations. I believe it can be done internally inside StandardInstrumentations.cpp. That is why I proposed the Instrument base class.
- Made legacy InstCombine pass to unconditionally report changing CFG
- New Pass Manager's InstCombine is changed to report changing CFG only if branches were swapped at least once
- Updated tests accordingly
Jun 10 2020
Good point! In other words, if DFSNum was a separate CFG base analysis (and it is as a part of DomTree), it must be invalidated.
@lebedev.ri This shows that making the PostDomTree better will not help.
I will stick to invalidating all CFG analyses in InstCombine only if successors are reordered. I believe we should do this to fix the bug. A better solution can be implemented later on.
added comment. Moved BP variable to its definition.
There is no way to update for successors switch. It would be a noop for the updater.