Page MenuHomePhabricator

yrouban (Yevgeny Rouban)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 4 2017, 1:39 AM (167 w, 5 d)

Recent Activity

Sun, Sep 13

yrouban committed rG88690a965892: [CodeGenPrepare] Fix zapping dead operands of assume (authored by yrouban).
[CodeGenPrepare] Fix zapping dead operands of assume
Sun, Sep 13, 9:48 PM
yrouban closed D87434: [CodeGenPrepare] Fix zapping dead operands of assume.
Sun, Sep 13, 9:47 PM · Restricted Project

Fri, Sep 11

yrouban abandoned D81089: Mark InstCombine as not preserving CFG.

See D84763.

Fri, Sep 11, 6:48 AM · Restricted Project
yrouban updated the summary of D87434: [CodeGenPrepare] Fix zapping dead operands of assume.
Fri, Sep 11, 1:49 AM · Restricted Project
yrouban updated the diff for D87434: [CodeGenPrepare] Fix zapping dead operands of assume.

RecursivelyDeleteTriviallyDeadInstructions() call is wrapped with resetIteratorIfInvalidatedWhileCalling().

Fri, Sep 11, 1:48 AM · Restricted Project
yrouban added a comment to D87434: [CodeGenPrepare] Fix zapping dead operands of assume.

Thanks. Can we just wrap it in resetIteratorIfInvalidatedWhileCalling like the call a few lines later? That seems less error prone.

Ok, but it will be less efficient.

Fri, Sep 11, 1:46 AM · Restricted Project
yrouban committed rG28012e00d80b: [NewPM] Introduce PreserveCFG check (authored by yrouban).
[NewPM] Introduce PreserveCFG check
Fri, Sep 11, 12:33 AM
yrouban closed D81558: [NewPM] Introduce PreserveCFG check.
Fri, Sep 11, 12:33 AM · Restricted Project

Thu, Sep 10

yrouban updated the diff for D81558: [NewPM] Introduce PreserveCFG check.

Addressed Fedor's comments.

Thu, Sep 10, 1:33 AM · Restricted Project
yrouban added inline comments to D81558: [NewPM] Introduce PreserveCFG check.
Thu, Sep 10, 1:32 AM · Restricted Project

Wed, Sep 9

yrouban requested review of D87434: [CodeGenPrepare] Fix zapping dead operands of assume.
Wed, Sep 9, 10:46 PM · Restricted Project
yrouban added a comment to D81558: [NewPM] Introduce PreserveCFG check.

Fedor, could you please review the New Pass Manager part of this patch as I addressed your comments?

Wed, Sep 9, 9:29 PM · Restricted Project

Fri, Sep 4

yrouban added a comment to D81558: [NewPM] Introduce PreserveCFG check.

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

Fri, Sep 4, 10:59 PM · Restricted Project

Thu, Sep 3

yrouban updated the diff for D81558: [NewPM] Introduce PreserveCFG check.
Thu, Sep 3, 9:54 PM · Restricted Project
yrouban updated the diff for D81558: [NewPM] Introduce PreserveCFG check.

a minor fix in BBGuard.

Thu, Sep 3, 9:40 PM · Restricted Project
yrouban updated the diff for D81558: [NewPM] Introduce PreserveCFG check.

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.
Addressed comments.
I will provide performance data soon.

Thu, Sep 3, 9:36 PM · Restricted Project
yrouban added inline comments to D86657: Add new hidden option -print-crash-IR that prints out IR that caused opt pipeline to crash.
Thu, Sep 3, 8:14 PM · Restricted Project
yrouban planned changes to D81558: [NewPM] Introduce PreserveCFG check.

I will collect performance data and fix for release builds.

Thu, Sep 3, 5:12 AM · Restricted Project

Wed, Sep 2

yrouban added inline comments to D86657: Add new hidden option -print-crash-IR that prints out IR that caused opt pipeline to crash.
Wed, Sep 2, 11:13 PM · Restricted Project
yrouban accepted D86360: Add new hidden option -print-changed which only reports changes to IR.

No further complains from me.
lgtm if the other reviewers' comments are addressed.

Wed, Sep 2, 7:59 PM · Restricted Project

Tue, Sep 1

yrouban added inline comments to D86657: Add new hidden option -print-crash-IR that prints out IR that caused opt pipeline to crash.
Tue, Sep 1, 9:24 PM · Restricted Project
yrouban added inline comments to D86360: Add new hidden option -print-changed which only reports changes to IR.
Tue, Sep 1, 9:06 PM · Restricted Project
yrouban added a comment to D81558: [NewPM] Introduce PreserveCFG check.

This looks very useful. Does it currently report any errors when enabled?

None.

Tue, Sep 1, 9:15 AM · Restricted Project
yrouban updated the diff for D81558: [NewPM] Introduce PreserveCFG check.

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.
Rebased.

Tue, Sep 1, 1:27 AM · Restricted Project

Sun, Aug 30

yrouban added inline comments to D86657: Add new hidden option -print-crash-IR that prints out IR that caused opt pipeline to crash.
Sun, Aug 30, 10:14 PM · Restricted Project

Thu, Aug 27

yrouban added inline comments to D86657: Add new hidden option -print-crash-IR that prints out IR that caused opt pipeline to crash.
Thu, Aug 27, 9:26 PM · Restricted Project

Wed, Aug 26

yrouban added inline comments to D86657: Add new hidden option -print-crash-IR that prints out IR that caused opt pipeline to crash.
Wed, Aug 26, 9:34 PM · Restricted Project

Sun, Aug 23

yrouban added inline comments to D86360: Add new hidden option -print-changed which only reports changes to IR.
Sun, Aug 23, 9:08 PM · Restricted Project

Aug 21 2020

yrouban committed rG18bc400f97a6: [NewPM][PassInstrumentation] Add PreservedAnalyses parameter to AfterPass*… (authored by yrouban).
[NewPM][PassInstrumentation] Add PreservedAnalyses parameter to AfterPass*…
Aug 21 2020, 2:11 AM
yrouban closed D81555: [NewPM][PassInstrumentation] Add PreservedAnalyses parameter to AfterPass* callbacks.
Aug 21 2020, 2:11 AM · Restricted Project

Aug 20 2020

yrouban committed rG7d9a16241fdd: [ADT] Allow IsSizeLessThanThresholdT for incomplete types. NFC (authored by yrouban).
[ADT] Allow IsSizeLessThanThresholdT for incomplete types. NFC
Aug 20 2020, 9:03 PM
yrouban closed D81554: [ADT] Allow IsSizeLessThanThresholdT for incomplete types. NFC.
Aug 20 2020, 9:03 PM · Restricted Project

Aug 13 2020

yrouban abandoned D84495: [InstCombine] Disable branch predicate canonicalization by default.
Aug 13 2020, 8:53 AM · Restricted Project
yrouban abandoned D84493: [InstCombine] Disable branch predicate canonicalization.
Aug 13 2020, 8:53 AM · Restricted Project
yrouban abandoned D84492: [SimplifyCFG] Enable branch predicate canonicalization by default.
Aug 13 2020, 8:52 AM · Restricted Project
yrouban abandoned D84491: [SimplifyCFG] Canonicalize branch predicates.
Aug 13 2020, 8:51 AM · Restricted Project

Aug 6 2020

yrouban added a comment to D84763: DomTree: Make PostDomTree immune to block successors swap.

This also broke the Polly buildbot twice: http://lab.llvm.org:8011/builders/polly-x86_64-linux/builds/3542 and http://lab.llvm.org:8011/builders/polly-x86_64-linux/builds/3547. Please also fix the Polly tests when you get notifications from the Polly buildbot.

ok. As I see, both failures have been fixed. Thanks @Meinersbur for fixing polly/test/Isl/Ast/alias_checks_with_empty_context.ll

Aug 6 2020, 9:21 PM · Restricted Project

Aug 5 2020

yrouban committed rGbc10888dcdda: DomTree: Make PostDomTree indifferent to block successors swap (authored by yrouban).
DomTree: Make PostDomTree indifferent to block successors swap
Aug 5 2020, 12:27 AM

Aug 4 2020

yrouban added inline comments to D81558: [NewPM] Introduce PreserveCFG check.
Aug 4 2020, 10:37 PM · Restricted Project
yrouban added a comment to D84763: DomTree: Make PostDomTree immune to block successors swap.

Thanks. I should have tested some more than just llvm.

Aug 4 2020, 10:29 PM · Restricted Project
yrouban committed rGc35585e209ef: DomTree: Make PostDomTree immune to block successors swap (authored by yrouban).
DomTree: Make PostDomTree immune to block successors swap
Aug 4 2020, 9:08 PM
yrouban closed D84763: DomTree: Make PostDomTree immune to block successors swap.
Aug 4 2020, 9:08 PM · Restricted Project
yrouban added a comment to D84763: DomTree: Make PostDomTree immune to block successors swap.

@nikic reported that the geomean regression is 0.1%.
@kuhar, could you please lgtm?
Please consider taking look at D81558 (PreserveCFG checker).

Aug 4 2020, 4:23 AM · Restricted Project

Aug 3 2020

yrouban updated the diff for D84763: DomTree: Make PostDomTree immune to block successors swap.

Reverted NodeOrderMap from SmallDenseMap to DenseMap.

Aug 3 2020, 4:33 AM · Restricted Project
yrouban updated the diff for D84763: DomTree: Make PostDomTree immune to block successors swap.

changed type of SuccOrder to SmallDenseMap<NodePtr, unsigned, 16>.

Aug 3 2020, 2:34 AM · Restricted Project
yrouban added inline comments to D84763: DomTree: Make PostDomTree immune to block successors swap.
Aug 3 2020, 2:32 AM · Restricted Project

Aug 2 2020

yrouban added a comment to D84763: DomTree: Make PostDomTree immune to block successors swap.

The code looks fine to me. Have you checked if compilation times are affected by this patch?

I have not checked compile time impact. I see 3 options:

  1. Land the patch and hope that any regression will be caught by @nikic's http://llvm-compile-time-tracker.com/
  2. Ask @nikic to check this particular patch before I land it
  3. Ask @nikic to add a custom perf/ branch for me to test this patch on my own
Aug 2 2020, 11:42 PM · Restricted Project
yrouban updated the diff for D84763: DomTree: Make PostDomTree immune to block successors swap.

added comment on SuccOrder parameter of runDFS().

Aug 2 2020, 11:35 PM · Restricted Project

Jul 30 2020

yrouban planned changes to D84491: [SimplifyCFG] Canonicalize branch predicates.
Jul 30 2020, 9:13 PM · Restricted Project
yrouban updated the diff for D81558: [NewPM] Introduce PreserveCFG check.

Rebased on top of D84772. So D84826 is not needed.
Fixed safety of printCFGdiff() to handle removed blocks.

Jul 30 2020, 5:05 AM · Restricted Project
yrouban abandoned D84826: [NewPM][PassInstrumentation] Add AfterPassSkipped callback.

See D84772.

Jul 30 2020, 5:02 AM · Restricted Project
yrouban updated the diff for D81555: [NewPM][PassInstrumentation] Add PreservedAnalyses parameter to AfterPass* callbacks.

rebased + addressed comments (minor)

Jul 30 2020, 4:59 AM · Restricted Project
yrouban added inline comments to D84772: [NewPM][PassInstrument] Add a new kind of before-pass callback that only get called if the pass is not skipped.
Jul 30 2020, 2:00 AM · Restricted Project
yrouban retitled D81555: [NewPM][PassInstrumentation] Add PreservedAnalyses parameter to AfterPass* callbacks from [NewPM][PassInstrumentation] Add PreservedAnalyses parameter to AfterPass* callbacks. NFC to [NewPM][PassInstrumentation] Add PreservedAnalyses parameter to AfterPass* callbacks.
Jul 30 2020, 1:59 AM · Restricted Project
yrouban updated the diff for D84763: DomTree: Make PostDomTree immune to block successors swap.

Addressed comments.
Added a test case with two reverse unreachable subgraphs.

Jul 30 2020, 1:50 AM · Restricted Project

Jul 29 2020

yrouban added a comment to D84763: DomTree: Make PostDomTree immune to block successors swap.

One more thing: should this be backported for the upcoming 11.0 release?

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 29 2020, 9:39 PM · Restricted Project
yrouban committed rG5d6cd61904aa: [LoopSimplifyCFG] Delete landing pads in dead exit blocks (authored by yrouban).
[LoopSimplifyCFG] Delete landing pads in dead exit blocks
Jul 29 2020, 4:37 AM
yrouban closed D84320: [LoopSimplifyCFG] Delete landing pads in dead exit blocks.
Jul 29 2020, 4:37 AM · Restricted Project

Jul 28 2020

yrouban added a reviewer for D84772: [NewPM][PassInstrument] Add a new kind of before-pass callback that only get called if the pass is not skipped: fedor.sergeev.
Jul 28 2020, 11:09 PM · Restricted Project
yrouban updated the diff for D81558: [NewPM] Introduce PreserveCFG check.

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.
Rebased.

Jul 28 2020, 11:00 PM · Restricted Project
yrouban requested review of D84826: [NewPM][PassInstrumentation] Add AfterPassSkipped callback.
Jul 28 2020, 10:44 PM · Restricted Project
yrouban updated the diff for D81555: [NewPM][PassInstrumentation] Add PreservedAnalyses parameter to AfterPass* callbacks.

just rebased.

Jul 28 2020, 10:26 PM · Restricted Project
yrouban added a comment to D84763: DomTree: Make PostDomTree immune to block successors swap.

Taking a few steps back, I'd love to see a cfg preservation check like D81558 ...

I'm about to update the patches.

Jul 28 2020, 10:18 PM · Restricted Project
yrouban requested review of D84763: DomTree: Make PostDomTree immune to block successors swap.
Jul 28 2020, 8:42 AM · Restricted Project

Jul 24 2020

yrouban added inline comments to D84320: [LoopSimplifyCFG] Delete landing pads in dead exit blocks.
Jul 24 2020, 9:35 AM · Restricted Project
yrouban planned changes to D84493: [InstCombine] Disable branch predicate canonicalization.

So in other words, instead of paying the price of recreating domtrees after instcombine,
we'll just shift the transforms to some other pass that already causes domtrees to be recreated, correct?

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.

Jul 24 2020, 5:15 AM · Restricted Project
yrouban added a comment to D84493: [InstCombine] Disable branch predicate canonicalization.

Thank you for pointing this out. I will try changing canonicalizeICmpPredicate(), canFreelyInvertAllUsersOf().

Jul 24 2020, 4:14 AM · Restricted Project
yrouban set the repository for D84492: [SimplifyCFG] Enable branch predicate canonicalization by default to rG LLVM Github Monorepo.
Jul 24 2020, 12:28 AM · Restricted Project
yrouban set the repository for D84491: [SimplifyCFG] Canonicalize branch predicates to rG LLVM Github Monorepo.
Jul 24 2020, 12:27 AM · Restricted Project
Herald added a project to D84495: [InstCombine] Disable branch predicate canonicalization by default: Restricted Project.
Jul 24 2020, 12:26 AM · Restricted Project
Herald added a project to D84493: [InstCombine] Disable branch predicate canonicalization: Restricted Project.
Jul 24 2020, 12:17 AM · Restricted Project
yrouban updated the summary of D84492: [SimplifyCFG] Enable branch predicate canonicalization by default.
Jul 24 2020, 12:07 AM · Restricted Project
Herald added a project to D84492: [SimplifyCFG] Enable branch predicate canonicalization by default: Restricted Project.
Jul 24 2020, 12:06 AM · Restricted Project

Jul 23 2020

Herald added a project to D84491: [SimplifyCFG] Canonicalize branch predicates: Restricted Project.
Jul 23 2020, 11:57 PM · Restricted Project

Jul 22 2020

Herald added a project to D84320: [LoopSimplifyCFG] Delete landing pads in dead exit blocks: Restricted Project.
Jul 22 2020, 6:36 AM · Restricted Project

Jul 21 2020

yrouban added a comment to D81554: [ADT] Allow IsSizeLessThanThresholdT for incomplete types. NFC.

@chandlerc could you please review or suggest someone who can do it.
This patch is needed for D81555 and D81558.

Jul 21 2020, 10:45 PM · Restricted Project

Jun 19 2020

yrouban committed rG6429471e8b76: [IR] Convert profile metadata in createCallMatchingInvoke() (authored by yrouban).
[IR] Convert profile metadata in createCallMatchingInvoke()
Jun 19 2020, 10:44 PM
yrouban closed D82071: [IR] Convert profile metadata in createCallMatchingInvoke().
Jun 19 2020, 10:43 PM · Restricted Project

Jun 18 2020

yrouban added a comment to D82071: [IR] Convert profile metadata in createCallMatchingInvoke().

Seems reasonable, but I don't know that much about profile info.. is it actually useful to have "branch_weights" metadata on a call? Isn't it enough that there's profile info on the branch towards the BB that has the call instruction? Or is this data used for something more?

Branch_weights are useful for call instructions as said in the LLVM LangRef (https://llvm.org/docs/BranchWeightMetadata.html).
Let us wait for @xur to comment.

Jun 18 2020, 4:52 AM · Restricted Project
yrouban closed D81449: [JumpThreading] Handle zero !prof branch_weights.

Landed as 707836ed4edb21e7133007f0200c3fd3a04d3409

Jun 18 2020, 1:03 AM · Restricted Project
yrouban added a comment to D81554: [ADT] Allow IsSizeLessThanThresholdT for incomplete types. NFC.

@chandlerc, @fedor.sergeev, @vvereschaka, @DaniilSuchkov, please review or suggest who can review.

Jun 18 2020, 1:03 AM · Restricted Project
yrouban created D82071: [IR] Convert profile metadata in createCallMatchingInvoke().
Jun 18 2020, 12:30 AM · Restricted Project

Jun 17 2020

yrouban added a comment to D80618: Extend InvokeInst !prof branch_weights metadata to unwind branches.

Thank you for reporting the case.
The bug is fixed with https://reviews.llvm.org/D81996 by Hans Wennborg.

Jun 17 2020, 11:11 PM · Restricted Project
yrouban updated subscribers of D81996: [IR] Don't copy profile metadata in createCallMatchingInvoke().
Jun 17 2020, 10:39 PM · Restricted Project

Jun 11 2020

yrouban committed rG707836ed4edb: [JumpThreading] Handle zero !prof branch_weights (authored by yrouban).
[JumpThreading] Handle zero !prof branch_weights
Jun 11 2020, 9:57 PM
yrouban added a comment to D81558: [NewPM] Introduce PreserveCFG check.

I'm not sure if introducing base for standard instrumentation classes (Instrument/Instruments) is a good change or not.
What problem does it solve?
Anyway that part seems to be rather orthogonal to the checker itself.
I would like to have it separated - either introduce Instrument/Instruments thing before the checker and then use it,
OR implement the checker the same way as other standard instrumentations are done (just as separate members) and only then refactor it all into Instrument/Instruments.

Ok.
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.

Jun 11 2020, 2:26 AM · Restricted Project
yrouban updated the diff for D81089: Mark InstCombine as not preserving CFG.
  • 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 11 2020, 1:14 AM · Restricted Project

Jun 10 2020

yrouban added a comment to D81089: Mark InstCombine as not preserving CFG.

Even if we agree the way the PostDominatorTree computes roots in cases involving infinite loops shouldn't depend on the ordering of successors, I'm not sure it's correct to say that instcombine "preserves the CFG". Even for the regular DominatorTree, I think reordering the successors invalidates DFS numbering (in the sense that it changes the result of getDFSNumIn()).

In terms of how the PostDominatorTree should decide what a "root" is, I'm not sure there's any good way to make that decision. Given an SCC which doesn't have any natural parent, you can pick any node in that SCC as the "fake root", and there isn't any natural reason to favor one node over another.

Really, the natural choice is to just follow the standard dominator algorithm and say the nodes are unreachable. Then for passes which want it, we can add a transform to mutate the CFG to introduce fake roots.

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.

Jun 10 2020, 10:40 PM · Restricted Project
yrouban updated the diff for D81449: [JumpThreading] Handle zero !prof branch_weights.

added comment. Moved BP variable to its definition.

Jun 10 2020, 10:06 PM · Restricted Project
yrouban added a comment to D81449: [JumpThreading] Handle zero !prof branch_weights.

@kazu, @nikic, @yamauchi, @ebrevnov, @efriedma, please review this small fix.

Jun 10 2020, 5:59 AM · Restricted Project
yrouban updated the summary of D81555: [NewPM][PassInstrumentation] Add PreservedAnalyses parameter to AfterPass* callbacks.
Jun 10 2020, 5:58 AM · Restricted Project
yrouban created D81558: [NewPM] Introduce PreserveCFG check.
Jun 10 2020, 5:58 AM · Restricted Project
yrouban added a comment to D81089: Mark InstCombine as not preserving CFG.

Which fold is at fault here?
Can it be simply taught to update domtree?

There is no way to update for successors switch. It would be a noop for the updater.

Jun 10 2020, 5:57 AM · Restricted Project
yrouban created D81555: [NewPM][PassInstrumentation] Add PreservedAnalyses parameter to AfterPass* callbacks.
Jun 10 2020, 5:57 AM · Restricted Project
yrouban added a reviewer for D81554: [ADT] Allow IsSizeLessThanThresholdT for incomplete types. NFC: DaniilSuchkov.
Jun 10 2020, 5:25 AM · Restricted Project
yrouban created D81554: [ADT] Allow IsSizeLessThanThresholdT for incomplete types. NFC.
Jun 10 2020, 5:25 AM · Restricted Project

Jun 9 2020

yrouban created D81449: [JumpThreading] Handle zero !prof branch_weights.
Jun 9 2020, 2:09 AM · Restricted Project

Jun 4 2020

yrouban committed rGdcfa78a4ccec: Extend InvokeInst !prof branch_weights metadata to unwind branches (authored by yrouban).
Extend InvokeInst !prof branch_weights metadata to unwind branches
Jun 4 2020, 2:08 AM
yrouban closed D80618: Extend InvokeInst !prof branch_weights metadata to unwind branches.
Jun 4 2020, 2:08 AM · Restricted Project
yrouban committed rG417bcb882767: [Instruction] Remove setProfWeight() (authored by yrouban).
[Instruction] Remove setProfWeight()
Jun 4 2020, 1:35 AM