Page MenuHomePhabricator

skatkov (Serguei Katkov)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 25 2017, 1:38 AM (98 w, 2 d)

Recent Activity

Tue, Dec 11

skatkov added a comment to D49519: [RegisterCoalescer] Delay live interval update work until the rematerialization for all the uses from the same def is done.

Hello @wmi, in our local fuzzing testing we've got the following assert:
src/include/llvm/ADT/IntervalMap.h:630: unsigned int llvm::IntervalMapImpl::LeafNode<KeyT, ValT, N, Traits>::insertFrom(unsigned int&, unsigned int, KeyT, KeyT, ValT) [with KeyT = llvm::SlotIndex; ValT = unsigned int; unsigned int N = 9u; Traits = llvm::IntervalMapInfo<llvm::SlotIndex>]: Assertion `!Traits::stopLess(b, a) && "Invalid interval"' failed.

Tue, Dec 11, 8:23 PM

Thu, Dec 6

skatkov accepted D55357: [LoopSimplifyCFG] Do not deal with loops with irreducible CFG inside.
Thu, Dec 6, 9:04 PM

Thu, Nov 29

skatkov abandoned D54517: [CGP] Limit Complex Addressing mode by number of BasicBlocks to traverse.

abandon in favor of D54932.

Thu, Nov 29, 12:44 AM

Wed, Nov 28

skatkov committed rL347839: [CGP] Improve compile time for complex addressing mode.
[CGP] Improve compile time for complex addressing mode
Wed, Nov 28, 10:48 PM
skatkov closed D54932: [CGP] Improve compile time for complex addressing mode.
Wed, Nov 28, 10:48 PM
skatkov added a comment to D54713: [SCEV] Guard movement of insertion point for loop-invariants.

@skatkov: About the loop unswitching suggestion, that sounds like a fine idea to me. But definitely outside the scope of this fix.

Absolutely agreed. It was just a sid comment.

(As an aside, other than a compiler-crash during vectorization (due to a deref of a null pointer), this is my first experience in the llvm vectorizer.  So I'm not familiar with how to go about adding the improvement you suggested.

I'm not sure whether you use a custom pipeline or not. But my point was that code came to vectorization pass seems strange because trivial loop unswitch optimization (provided by LoopUnswitch pass) could hoist the invariant check. By some reason it did not happen.

It's experience I'd like to gain, so I'll consider it as possible future work. But to make sure I'm not misleading you, I don't have time to work on this in the immediate future.)

Not a problem at all.

Wed, Nov 28, 10:21 PM
skatkov added inline comments to D54713: [SCEV] Guard movement of insertion point for loop-invariants.
Wed, Nov 28, 9:39 PM
skatkov abandoned D54289: [CGP] Sink invariant load to its use.

This is invalid patch.

Wed, Nov 28, 8:51 PM
skatkov updated subscribers of D54932: [CGP] Improve compile time for complex addressing mode.
Wed, Nov 28, 8:02 PM
skatkov accepted D54713: [SCEV] Guard movement of insertion point for loop-invariants.

LGTM. If Max or Sanjoy does not object in a, say, one day, feel free to land.

Wed, Nov 28, 6:59 PM

Tue, Nov 27

skatkov added inline comments to D54713: [SCEV] Guard movement of insertion point for loop-invariants.
Tue, Nov 27, 11:38 PM
skatkov added inline comments to D54713: [SCEV] Guard movement of insertion point for loop-invariants.
Tue, Nov 27, 11:10 PM
skatkov updated the diff for D54932: [CGP] Improve compile time for complex addressing mode.

Comments updated.

Tue, Nov 27, 6:59 PM
skatkov added a comment to D54932: [CGP] Improve compile time for complex addressing mode.

Agreed, I missed comments in hurry. Will update a patch...

Tue, Nov 27, 5:51 PM

Mon, Nov 26

skatkov added a comment to D54517: [CGP] Limit Complex Addressing mode by number of BasicBlocks to traverse.

Hi @john.brawn, could you please take a look at https://reviews.llvm.org/D54932. It makes a compile time improvement and I guess we will not need this patch for a while.

Mon, Nov 26, 11:03 PM
skatkov created D54932: [CGP] Improve compile time for complex addressing mode.
Mon, Nov 26, 11:00 PM
skatkov added inline comments to D54713: [SCEV] Guard movement of insertion point for loop-invariants.
Mon, Nov 26, 6:29 PM
skatkov added a comment to D54517: [CGP] Limit Complex Addressing mode by number of BasicBlocks to traverse.

Looks OK as a fix for the reported bug, with one minor nitpick. Taking a look at PR39625 it looks like there's an underlying problem where we insert a load of useless placeholders, e.g. for

declare void @otherfn()

define i32 @fn(i32* %arg1, i32* %arg2) {
entry:
  %gep1 = getelementptr i32, i32* %arg1, i32 4
  %gep2 = getelementptr i32, i32* %arg1, i32 8
  br i1 undef, label %a1, label %b1

a1:
  call void @otherfn()
  br label %middle

b1:
  call void @otherfn()
  br label %middle

middle:
  br i1 undef, label %a2, label %b2

a2:
  call void @otherfn()
  br label %end

b2:
  call void @otherfn()
  br label %end

end:
  %phi = phi i32* [ %gep1, %a2 ], [ %gep2, %b2 ]
  %val = load i32, i32* %phi, align 4
  ret i32 %val
}

we insert a total of 9 placeholders, but actually we need only one and the rest get deleted later. So I think there's something that could be done to improve that, but that's no reason not to do this fix.

Mon, Nov 26, 6:25 PM
skatkov added inline comments to D54713: [SCEV] Guard movement of insertion point for loop-invariants.
Mon, Nov 26, 6:14 PM
skatkov added a comment to D54517: [CGP] Limit Complex Addressing mode by number of BasicBlocks to traverse.

FYI: in parallel there is an idea how to simplify algorithm significantly. I'll try to implement it...

Mon, Nov 26, 12:45 AM

Thu, Nov 22

skatkov accepted D54841: [LoopSimplifyCFG] Don't delete LCSSA Phis.

Feel free to mention @dmgreen in description who found this bug.

Thu, Nov 22, 10:21 PM
skatkov added inline comments to D54713: [SCEV] Guard movement of insertion point for loop-invariants.
Thu, Nov 22, 10:11 PM
skatkov added a comment to D54517: [CGP] Limit Complex Addressing mode by number of BasicBlocks to traverse.

Thank you, Bjorn.

Thu, Nov 22, 4:50 PM

Tue, Nov 20

skatkov added a comment to D54517: [CGP] Limit Complex Addressing mode by number of BasicBlocks to traverse.

Basically I think the code looks ok. However, I'm not so familiar with this algorithm so it is hard to comment about the actual solution.

My understanding is that you introduce a threshold, and if the size of the TraverseOrder vector grows past the threshold we bail out from findCommon. So what is the impact of this?
I assume it means that we limit the amount of "Complex Addressing mode" optimizations somehow. Is this limit only hit for "large" programs? When compiling a program that hits the threshold, do we lose some optimizations or will the amount of Complex Addressing mode optimizations in such a program be reduced significantly?

How did you choose the current threshold? (that is probably something people want to know when trying to finetune this 4 years from now, so it could be nice to say something about it in the commit msg even if it just is an estimate)

Hi Bjorn, thank you for looking into this patch.

Tue, Nov 20, 7:57 PM

Mon, Nov 19

skatkov planned changes to D54289: [CGP] Sink invariant load to its use.

Basing on https://bugs.llvm.org/show_bug.cgi?id=23603 I think this patch is incorrect.

Mon, Nov 19, 8:42 PM
skatkov added a reviewer for D54289: [CGP] Sink invariant load to its use: sanjoy.
Mon, Nov 19, 8:41 PM
skatkov added a comment to D54517: [CGP] Limit Complex Addressing mode by number of BasicBlocks to traverse.

ping.

Mon, Nov 19, 8:28 PM

Nov 14 2018

skatkov created D54517: [CGP] Limit Complex Addressing mode by number of BasicBlocks to traverse.
Nov 14 2018, 1:54 AM

Nov 12 2018

skatkov added a comment to D54289: [CGP] Sink invariant load to its use.

There's also a sinking pass, why isn't this done there?

Sink pass is written in the way it sink instructions only to nearest successors while for invariant load I want to move it anywhere in CFG...

Ok, I was wrong Sink pass iterates. If I add a support for invariant.load it can sink it but it definitely cannot sink it through diamond or loop. In parallel I'm going to take a look deeper whether I can handle my case in Sink pass...

Nov 12 2018, 9:59 PM

Nov 11 2018

skatkov accepted D54007: Use a data structure better suited for large sets in SimplificationTracker..

LGTM as well

Nov 11 2018, 7:35 PM

Nov 8 2018

skatkov added a comment to D54289: [CGP] Sink invariant load to its use.

There's also a sinking pass, why isn't this done there?

Sink pass is written in the way it sink instructions only to nearest successors while for invariant load I want to move it anywhere in CFG...

Ok, I was wrong Sink pass iterates. If I add a support for invariant.load it can sink it but it definitely cannot sink it through diamond or loop. In parallel I'm going to take a look deeper whether I can handle my case in Sink pass...

Nov 8 2018, 9:25 PM
skatkov updated the diff for D54289: [CGP] Sink invariant load to its use.

Handled comments. Fow a while I'll double check Sink.cpp...

Nov 8 2018, 8:54 PM
skatkov added a comment to D54289: [CGP] Sink invariant load to its use.

There's also a sinking pass, why isn't this done there?

Nov 8 2018, 8:15 PM
skatkov added a comment to D54289: [CGP] Sink invariant load to its use.

Hi @arsenm, could you please take a look into this patch? If I turn the option on I see some AMDGPU test failures. I'm not an expert in AMDGPU asm/arch but it seems that 4 invariant loads are added in the beginning of pipeline in entry block and I hoist one of them to colder block. As a result as I understand instead of loading of two load at once this "double-load" is split into two loads. I'm not sure that it is a beneficial for this platform. So I wonder whether there is a easy way to prohibit optimization for such cases for this platform?
Even if there is no easy way to do that I'd like to land it with option off by default.

Nov 8 2018, 7:59 PM
skatkov created D54289: [CGP] Sink invariant load to its use.
Nov 8 2018, 7:53 PM

Nov 7 2018

skatkov added a comment to D54223: [SimpleLoopUnswitch] adding cost multiplier to cap exponential unswitch with.

In general I think it is what we can go/start with.
This formula seems to be a bit pessimistic. Specifically not every candidate creates sibling loops. Only if all successors is in loop or there are sub-loops before unswitch candidate. So applying the power multiplier for this candidates seems unreasonable.
Also it is possible that several unswitch candidates can be done at one shot (the same condition), in this case the number of siblings will not grow so fast.
But these improvements can be done as a follow-up if needed.

Nov 7 2018, 7:06 PM
skatkov added a comment to D54007: Use a data structure better suited for large sets in SimplificationTracker..

Just a note, there is a specific usage of this data structure.
Phi nodes are inserted in the initial step without any removals and then only removals are used.

So we should not see something like delete-insert.
So while data structure really suffers from what Bjorn wrote but in this specific application of this data structure I do not expect any problems...

Ok. Just worried that someone might find this set implementation nifty, trying to reuse it in some other part of the code or moving it to ADT, without considering this limitation regarding delete-insert.

Completely agreed!

Nov 7 2018, 1:12 AM
skatkov added a comment to D54007: Use a data structure better suited for large sets in SimplificationTracker..

Just a note, there is a specific usage of this data structure.
Phi nodes are inserted in the initial step without any removals and then only removals are used.

Nov 7 2018, 12:33 AM

Nov 6 2018

skatkov added a comment to D54007: Use a data structure better suited for large sets in SimplificationTracker..

LGTM, please wait a bit and if Bjorn does not object, feel free to land.

Nov 6 2018, 6:26 PM

Nov 5 2018

skatkov added a comment to D54021: [LoopSimplifyCFG] Teach LoopSimplifyCFG to constant-fold branches and switches.

Also Max, please consider split this patch into two ones. One just NFC patch with analysis and update tests to check the output of dump and then patches which does modifications.

Nov 5 2018, 7:14 PM
skatkov added inline comments to D54021: [LoopSimplifyCFG] Teach LoopSimplifyCFG to constant-fold branches and switches.
Nov 5 2018, 6:48 PM

Oct 15 2018

skatkov accepted D53270: [NewPM] implement SCC printing for -print-before-all/-print-after-all.
Oct 15 2018, 12:53 AM
skatkov added inline comments to D53270: [NewPM] implement SCC printing for -print-before-all/-print-after-all.
Oct 15 2018, 12:40 AM
skatkov added inline comments to D53270: [NewPM] implement SCC printing for -print-before-all/-print-after-all.
Oct 15 2018, 12:21 AM

Sep 10 2018

skatkov committed rL341896: [LICM] Avoid duplicate work during building AliasSetTracker.
[LICM] Avoid duplicate work during building AliasSetTracker
Sep 10 2018, 9:09 PM
skatkov closed D51715: [LICM] Avoid duplicate work during building AliasSetTracker.
Sep 10 2018, 9:08 PM
skatkov accepted D51850: [IndVars][NFC] Refactor to make modifications of Changed transparent.
Sep 10 2018, 7:08 PM

Sep 9 2018

skatkov accepted D51779: [IndVars] Set Changed if rewriteFirstIterationLoopExitValues changes IR. PR38863.
Sep 9 2018, 10:18 PM
skatkov accepted D51777: [IndVars] Set Changed if sinkUnusedInvariants changes IR. PR38863.
Sep 9 2018, 10:15 PM
skatkov added a comment to D51779: [IndVars] Set Changed if rewriteFirstIterationLoopExitValues changes IR. PR38863.

The same comment as in D51777, The semantics of the patch itself lgtm.

Sep 9 2018, 6:21 PM
skatkov added a comment to D51777: [IndVars] Set Changed if sinkUnusedInvariants changes IR. PR38863.

I see that IndVarsSimplify uses class field Changed. You propose to use Changed as return value. I just want to see the same approach through the whole IndVars...

Sep 9 2018, 6:19 PM

Sep 7 2018

skatkov accepted D51770: [IndVars] Set Changed when we delete dead instructions. PR38855.

LGTM with follow-up handling of Changed. If you do not have cycles for handling Changed, please file a bug at least.

Sep 7 2018, 12:19 AM

Sep 6 2018

skatkov added inline comments to D51770: [IndVars] Set Changed when we delete dead instructions. PR38855.
Sep 6 2018, 11:30 PM
skatkov updated the diff for D51715: [LICM] Avoid duplicate work during building AliasSetTracker.

is it better?

Sep 6 2018, 11:25 PM
skatkov added inline comments to D51715: [LICM] Avoid duplicate work during building AliasSetTracker.
Sep 6 2018, 7:18 PM
skatkov added inline comments to D51715: [LICM] Avoid duplicate work during building AliasSetTracker.
Sep 6 2018, 6:43 PM

Sep 5 2018

skatkov created D51715: [LICM] Avoid duplicate work during building AliasSetTracker.
Sep 5 2018, 8:51 PM

Sep 3 2018

skatkov accepted D51535: [PassTiming] reporting time-passes separately for multiple pass instances of the same pass.
Sep 3 2018, 5:07 PM

Sep 2 2018

skatkov added a comment to D51535: [PassTiming] reporting time-passes separately for multiple pass instances of the same pass.

Could you please add a test producing something with #2 and #3... I guess any loop pass for function with 2-3 loops should work.

Sep 2 2018, 7:00 PM

Aug 20 2018

skatkov added a comment to D50554: [ValueTracking] Accept vectors of pointer in GetUnderlyingObject utility.

Verifier fix landed, can we proceed with this change?

Aug 20 2018, 9:39 PM
skatkov committed rL340249: [IR Verifier] Do not allow bitcast of pointer to vector of pointers and vice….
[IR Verifier] Do not allow bitcast of pointer to vector of pointers and vice…
Aug 20 2018, 9:28 PM
skatkov closed D50886: [IR Verifier] Do not allow bitcast of pointer to vector of pointers and vice versa..
Aug 20 2018, 9:28 PM

Aug 19 2018

skatkov updated the diff for D50886: [IR Verifier] Do not allow bitcast of pointer to vector of pointers and vice versa..
Aug 19 2018, 10:30 PM
skatkov added inline comments to D50886: [IR Verifier] Do not allow bitcast of pointer to vector of pointers and vice versa..
Aug 19 2018, 8:37 PM

Aug 17 2018

skatkov added a child revision for D50886: [IR Verifier] Do not allow bitcast of pointer to vector of pointers and vice versa.: D50554: [ValueTracking] Accept vectors of pointer in GetUnderlyingObject utility.
Aug 17 2018, 1:59 AM
skatkov added a parent revision for D50554: [ValueTracking] Accept vectors of pointer in GetUnderlyingObject utility: D50886: [IR Verifier] Do not allow bitcast of pointer to vector of pointers and vice versa..
Aug 17 2018, 1:59 AM
skatkov updated the diff for D50554: [ValueTracking] Accept vectors of pointer in GetUnderlyingObject utility.

Test is updated to be valid.

Aug 17 2018, 1:59 AM
skatkov created D50886: [IR Verifier] Do not allow bitcast of pointer to vector of pointers and vice versa..
Aug 17 2018, 1:56 AM

Aug 16 2018

skatkov added inline comments to D50554: [ValueTracking] Accept vectors of pointer in GetUnderlyingObject utility.
Aug 16 2018, 11:41 PM
skatkov added inline comments to D50554: [ValueTracking] Accept vectors of pointer in GetUnderlyingObject utility.
Aug 16 2018, 11:26 PM
skatkov added a comment to D50554: [ValueTracking] Accept vectors of pointer in GetUnderlyingObject utility.

ping. Can I do anything to proceed with this change?

Aug 16 2018, 10:25 PM

Aug 12 2018

skatkov added inline comments to D50558: [MustExecute] Fix algorithmic bug in isGuaranteedToExecute. PR38514.
Aug 12 2018, 8:27 PM

Aug 10 2018

skatkov updated the diff for D50554: [ValueTracking] Accept vectors of pointer in GetUnderlyingObject utility.

Thank you! I've added two unit tests.

Aug 10 2018, 3:22 AM
skatkov added a comment to D50554: [ValueTracking] Accept vectors of pointer in GetUnderlyingObject utility.

Thank you for your comment. Unfortunately till now I failed to create a test showing the benefit from this change in a pass (I tried). I can make one more try.
The problem I'm trying to solve is the following: in downstream pass I have a BitCast instruction which is actually originated from allocation but has a vector type gotten using gep + bitcast from allocation.
I'd like to know the fact that it is an underlying allocation but now utility function just ignore instructions with vector of pointers.

Aug 10 2018, 1:40 AM
skatkov retitled D50554: [ValueTracking] Accept vectors of pointer in GetUnderlyingObject utility from [ValueTracking] Accept vector of pointer in GetUnderlyingObject utility to [ValueTracking] Accept vectors of pointer in GetUnderlyingObject utility.
Aug 10 2018, 12:33 AM
skatkov created D50554: [ValueTracking] Accept vectors of pointer in GetUnderlyingObject utility.
Aug 10 2018, 12:31 AM

Aug 8 2018

skatkov accepted D50489: [LICM] hoist fences out of loops w/o memory operations.
Aug 8 2018, 9:57 PM

Jul 1 2018

skatkov accepted D48627: [ImplicitNullChecks] Check for rewrite of register used in 'test' instruction.
Jul 1 2018, 7:30 PM

Jun 27 2018

skatkov added a comment to D48627: [ImplicitNullChecks] Check for rewrite of register used in 'test' instruction.

Test, actually can be simplified bit if you do not mind.

Jun 27 2018, 10:02 PM
skatkov added inline comments to D48627: [ImplicitNullChecks] Check for rewrite of register used in 'test' instruction.
Jun 27 2018, 10:00 PM

Jun 4 2018

skatkov accepted D47371: [BPI] Apply invoke heuristic before loop branch heuristic.

LGTM. If no one objects in 1-2 days, feel free to land.

Jun 4 2018, 7:31 PM

Jun 3 2018

skatkov committed rL333864: [InstCombine] Fix div handling.
[InstCombine] Fix div handling
Jun 3 2018, 7:58 PM
skatkov closed D47576: [InstCombine] Fix div handling.
Jun 3 2018, 7:58 PM

May 31 2018

skatkov added inline comments to D47576: [InstCombine] Fix div handling.
May 31 2018, 6:15 PM
skatkov updated the diff for D47576: [InstCombine] Fix div handling.

Please take a look.

May 31 2018, 6:13 PM
skatkov created D47576: [InstCombine] Fix div handling.
May 31 2018, 1:44 AM

May 22 2018

skatkov committed rL333063: SafepointIRVerifier is made unreachable block tolerant.
SafepointIRVerifier is made unreachable block tolerant
May 22 2018, 10:58 PM
skatkov closed D47011: SafepointIRVerifier is made unreachable block tolerant.
May 22 2018, 10:58 PM

May 17 2018

skatkov committed rL332695: [LICM] Extend the MustExecute scope.
[LICM] Extend the MustExecute scope
May 17 2018, 10:03 PM
skatkov closed D46996: [LICM] Extend the MustExecute scope.
May 17 2018, 10:02 PM
skatkov updated the diff for D46996: [LICM] Extend the MustExecute scope.

Before landing.

May 17 2018, 8:29 PM

May 16 2018

skatkov created D46996: [LICM] Extend the MustExecute scope.
May 16 2018, 8:15 PM

May 9 2018

skatkov committed rL331950: [SCEV] Add missed Test for rL331949..
[SCEV] Add missed Test for rL331949.
May 9 2018, 6:46 PM
skatkov committed rL331949: SCEV] Do not use induction in isKnownPredicate for simplification umax..
SCEV] Do not use induction in isKnownPredicate for simplification umax.
May 9 2018, 6:44 PM
skatkov closed D46046: [SCEV] Do not use induction in isKnownPredicate for simplification umax.
May 9 2018, 6:44 PM

May 6 2018

skatkov added a comment to D46046: [SCEV] Do not use induction in isKnownPredicate for simplification umax.

@sanjoy, are you ok with the last change?

May 6 2018, 6:33 PM

May 2 2018

skatkov updated the diff for D46046: [SCEV] Do not use induction in isKnownPredicate for simplification umax.

Agreed with Max. Restriction of recursion between isKnownPredicate and simplification of umax is too complex.

May 2 2018, 9:42 PM

Apr 27 2018

skatkov committed rL331103: [SCEV] Touch the unsused stats variables for product build..
[SCEV] Touch the unsused stats variables for product build.
Apr 27 2018, 11:45 PM
skatkov committed rL331099: [SCEV] Reduce the number of invocation to non trivial getExact function.
[SCEV] Reduce the number of invocation to non trivial getExact function
Apr 27 2018, 8:58 PM
skatkov closed D46178: [SCEV] Reduce the number of invocation to non trivial getExact function.
Apr 27 2018, 8:58 PM