Page MenuHomePhabricator

skatkov (Serguei Katkov)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Sun, Mar 10

skatkov accepted D59106: [CodeGen] Add MMOs to statepoint nodes during SelectionDAG.

lgtm.

Sun, Mar 10, 11:44 PM · Restricted Project

Jan 24 2019

skatkov abandoned D56534: [Verifier] Add verification of unaligned atomic load/store.

Abandon the review.

Jan 24 2019, 10:20 PM

Jan 15 2019

skatkov committed rL351295: [InstCombine]Avoid introduction of unaligned mem access.
[InstCombine]Avoid introduction of unaligned mem access
Jan 15 2019, 8:40 PM
skatkov closed D56582: [InstCombine]Avoid introduction of unaligned mem access.
Jan 15 2019, 8:40 PM

Jan 13 2019

skatkov accepted D56120: [BasicBlockUtils] Generalize DeleteDeadBlock to deal with multiple dead blocks.
Jan 13 2019, 11:19 PM
skatkov updated the diff for D56582: [InstCombine]Avoid introduction of unaligned mem access.

Philip, I'm not sure that I completely follow your comment about spirit of the tests but I've added a coverage for size/alignment.
I hope it will be enough.

Jan 13 2019, 6:15 PM

Jan 11 2019

skatkov created D56582: [InstCombine]Avoid introduction of unaligned mem access.
Jan 11 2019, 1:03 AM

Jan 10 2019

skatkov created D56534: [Verifier] Add verification of unaligned atomic load/store.
Jan 10 2019, 12:22 AM

Dec 27 2018

skatkov added a comment to D55867: [RegisterCoalescer] dst register's live interval needs to be updated when merging a src register in ToBeUpdated set.

Ping, any progress on this?

Dec 27 2018, 2:52 AM

Dec 18 2018

skatkov added a comment to D55867: [RegisterCoalescer] dst register's live interval needs to be updated when merging a src register in ToBeUpdated set.

I've verified that this patch fixes the original problem.

Dec 18 2018, 7:48 PM

Dec 17 2018

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.

I was able to create a pure LLVM reproducer for this issue and filed a bug https://bugs.llvm.org/show_bug.cgi?id=40061.
Could you please take a look into it?

Dec 17 2018, 9:15 PM

Dec 11 2018

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.

Dec 11 2018, 8:23 PM

Dec 6 2018

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

Nov 29 2018

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

abandon in favor of D54932.

Nov 29 2018, 12:44 AM

Nov 28 2018

skatkov committed rL347839: [CGP] Improve compile time for complex addressing mode.
[CGP] Improve compile time for complex addressing mode
Nov 28 2018, 10:48 PM
skatkov closed D54932: [CGP] Improve compile time for complex addressing mode.
Nov 28 2018, 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.

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

This is invalid patch.

Nov 28 2018, 8:51 PM
skatkov updated subscribers of D54932: [CGP] Improve compile time for complex addressing mode.
Nov 28 2018, 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.

Nov 28 2018, 6:59 PM

Nov 27 2018

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

Comments updated.

Nov 27 2018, 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...

Nov 27 2018, 5:51 PM

Nov 26 2018

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.

Nov 26 2018, 11:03 PM
skatkov created D54932: [CGP] Improve compile time for complex addressing mode.
Nov 26 2018, 11:00 PM
skatkov added inline comments to D54713: [SCEV] Guard movement of insertion point for loop-invariants.
Nov 26 2018, 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.

Nov 26 2018, 6:25 PM
skatkov added inline comments to D54713: [SCEV] Guard movement of insertion point for loop-invariants.
Nov 26 2018, 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...

Nov 26 2018, 12:45 AM

Nov 22 2018

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

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

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

Thank you, Bjorn.

Nov 22 2018, 4:50 PM

Nov 20 2018

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.

Nov 20 2018, 7:57 PM

Nov 19 2018

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.

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

ping.

Nov 19 2018, 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