Page MenuHomePhabricator

fedor.sergeev (Fedor Sergeev)
User

Projects

User does not belong to any projects.

User Details

User Since
Feb 10 2017, 1:36 AM (96 w, 4 d)

Recent Activity

Wed, Dec 12

fedor.sergeev added inline comments to D55605: [NewPM] Port Msan: Alternative approach using a module to create the global constructor.
Wed, Dec 12, 2:25 PM

Tue, Dec 11

fedor.sergeev added a comment to D55278: [NewPM] -print-module-scope -print-after now prints module even after invalidated Loop/SCC.

ping?

Tue, Dec 11, 12:20 PM
fedor.sergeev committed rL348887: [NewPM] fixing asserts on deleted loop in -print-after-all.
[NewPM] fixing asserts on deleted loop in -print-after-all
Tue, Dec 11, 11:09 AM
fedor.sergeev closed D54740: [NewPM] fixing asserts on deleted loop in -print-after-all.
Tue, Dec 11, 11:09 AM
fedor.sergeev added a comment to D54740: [NewPM] fixing asserts on deleted loop in -print-after-all.

The invalid handle isn't good for accessing anything, but it can still identify a unit.

Not really. If you say have a Loop* pointer, which is freed and then another Loop allocated happens to be just at that pointer
then you dont have a way to identify whether it is the old Loop or the new one.
Presumably that does not happen now...

Tue, Dec 11, 4:10 AM

Mon, Dec 10

fedor.sergeev added a comment to D54740: [NewPM] fixing asserts on deleted loop in -print-after-all.

The IRUnit is invalid, but the handle still exists, right?

Invalid handle might be totally invalid, you can not use it for anything.
I do remember having accesses to freed memory when accessing either Loop or SCC, dont remember which one.
Even looking at the address might be not that sane if it was freed and perhaps something else allocated at that place.

Mon, Dec 10, 1:50 PM
fedor.sergeev added a comment to D54740: [NewPM] fixing asserts on deleted loop in -print-after-all.

In case the IRUnit has been invalidated, can we still pass it to the callback? When I think of instrumentations that do more complicated things than printing, it's highly likely that they would like to be notified when an IRUnit handle becomes invalid. Which requires passing the handle. If we are passing the handle, though, we need to emphasize that the unit isn't accessible through the handle anymore. Maybe by passing void* instead, or some tag type Invalidated<T> { void* Handle; }. The latter can go through the any and allow deducing the actual IRUnit type.

Mon, Dec 10, 1:26 PM

Fri, Dec 7

fedor.sergeev updated the diff for D54740: [NewPM] fixing asserts on deleted loop in -print-after-all.

removing unneeded asserts

Fri, Dec 7, 9:12 AM
fedor.sergeev added inline comments to D54740: [NewPM] fixing asserts on deleted loop in -print-after-all.
Fri, Dec 7, 9:08 AM
fedor.sergeev added inline comments to D54025: [LoopSimplifyCFG] Delete dead exiting edges.
Fri, Dec 7, 6:30 AM
fedor.sergeev accepted D54025: [LoopSimplifyCFG] Delete dead exiting edges.

LGTM.

Fri, Dec 7, 6:25 AM
fedor.sergeev added a comment to D55381: [llvm-tapi] Don't try to override SequenceTraits for std::string.

My fix ended up triggering some warnings in the bots that don't really have a reasonable fix so I reverted it. This is probably the best solution for the time being. lgtm

So, whats the plan here?
ElfYamlTextAPI.YAMLWritesNoTBESyms continues to fail in my local builds with LLVM_LINK_LLVM_DYLIB...

Fri, Dec 7, 4:48 AM

Thu, Dec 6

fedor.sergeev added a comment to D55278: [NewPM] -print-module-scope -print-after now prints module even after invalidated Loop/SCC.

What exactly does this change solve?

updated the summary.

Thu, Dec 6, 12:45 PM
fedor.sergeev retitled D55278: [NewPM] -print-module-scope -print-after now prints module even after invalidated Loop/SCC from [NewPM] -print-module-scope now prints module even after invalidated Loop/SCC to [NewPM] -print-module-scope -print-after now prints module even after invalidated Loop/SCC.
Thu, Dec 6, 12:45 PM
fedor.sergeev updated the diff for D55278: [NewPM] -print-module-scope -print-after now prints module even after invalidated Loop/SCC.

minor cleanup

Thu, Dec 6, 3:14 AM

Wed, Dec 5

fedor.sergeev added a comment to D54394: [WIP][NewPM] Port msan.
  • Make a free function for creating the global constructor function
  • The opt tool calls this function when passing a command line option

What is the benefit of having free function compared to having a module-level pass that does the same?

Wed, Dec 5, 6:12 AM

Tue, Dec 4

fedor.sergeev added a reviewer for D51207: Introduce llvm.experimental.widenable_condition intrinsic: chandlerc.
Tue, Dec 4, 7:51 PM
fedor.sergeev updated the diff for D55278: [NewPM] -print-module-scope -print-after now prints module even after invalidated Loop/SCC.

stack handling fixed (AfterPass now pops stack as well)

Tue, Dec 4, 9:37 AM
fedor.sergeev planned changes to D55278: [NewPM] -print-module-scope -print-after now prints module even after invalidated Loop/SCC.

oops, this needs a small update

Tue, Dec 4, 9:10 AM
fedor.sergeev added a child revision for D54740: [NewPM] fixing asserts on deleted loop in -print-after-all: D55278: [NewPM] -print-module-scope -print-after now prints module even after invalidated Loop/SCC.
Tue, Dec 4, 8:49 AM
fedor.sergeev added a parent revision for D55278: [NewPM] -print-module-scope -print-after now prints module even after invalidated Loop/SCC: D54740: [NewPM] fixing asserts on deleted loop in -print-after-all.
Tue, Dec 4, 8:49 AM
fedor.sergeev created D55278: [NewPM] -print-module-scope -print-after now prints module even after invalidated Loop/SCC.
Tue, Dec 4, 8:46 AM
fedor.sergeev updated the diff for D54740: [NewPM] fixing asserts on deleted loop in -print-after-all.

slight cleanup in anticipation of a followup that handles invalidated printing

Tue, Dec 4, 8:39 AM
fedor.sergeev added inline comments to D54740: [NewPM] fixing asserts on deleted loop in -print-after-all.
Tue, Dec 4, 6:19 AM

Mon, Dec 3

fedor.sergeev added a comment to D54337: [ASan] Make AddressSanitizer a ModulePass.

If asan doesn't modify the module for this ('this' meaning the Mapping and the GlobalsMD here), than this is an excellent candidate for an analysis.

When you say analysis, do you mean like the TargetLibraryInfoWrapperPass that AddressSanitizer is dependent on? Not sure what the difference between passes and analyses are, but if the purpose of TargetLibraryInfoWrapperPass is to just pass a TargetLibraryInfo to AddressSantizer, then it seems a new WrapperPass could similarly be made for this initialized data as a custom class made specifically for ASan.

Mon, Dec 3, 1:06 PM · Restricted Project
fedor.sergeev added a comment to D54337: [ASan] Make AddressSanitizer a ModulePass.

I am not an expert or have a good context, please excuse my trespassing, just as a guy reading the code and trying to understand, this is my understanding:

  1. GlobalsMD.init(M); is the only expensive part of the AddressSanitizer initialization.
  2. All data in GlobalsMD is obtained from a module, in GlobalsMetadata::init(Module& M).
  3. (Maybe this is not obvious) Function objects have a reference to their module via Function::getParent() method.
  4. Therefore, every expensive data that the function pass for AddressSanitizer needs, can be stored in Module objects:

Up to this point it kinda makes sense.
However, this suggestion of storing supplementary passes information into the IR itself does not sound like a good design decision.
A typical solution for LLVM to store supporting information is to introduce an Analysis, so it kinda floats around the IR, but
is not being stored directly into it.

Mon, Dec 3, 12:47 PM · Restricted Project
fedor.sergeev committed rL348144: Fixing -print-module-scope for legacy SCC passes.
Fixing -print-module-scope for legacy SCC passes
Mon, Dec 3, 6:51 AM
fedor.sergeev closed D54793: Fixing -print-module-scope for legacy SCC passes.
Mon, Dec 3, 6:51 AM
fedor.sergeev added a comment to D54848: [PM] Port LoadStoreVectorizer to the new pass manager..

Btw, @markus, you can go push this thing.

Mon, Dec 3, 6:01 AM
fedor.sergeev added inline comments to D54740: [NewPM] fixing asserts on deleted loop in -print-after-all.
Mon, Dec 3, 5:56 AM

Sun, Dec 2

fedor.sergeev added a comment to D54740: [NewPM] fixing asserts on deleted loop in -print-after-all.

ping?

Sun, Dec 2, 1:26 PM
fedor.sergeev accepted D55168: [test] Fix use of 'sort -b' in SimpleLoopUnswitch on NetBSD.
Sun, Dec 2, 7:48 AM
fedor.sergeev added a comment to D55168: [test] Fix use of 'sort -b' in SimpleLoopUnswitch on NetBSD.

i.e. the problem is that 'depth 2' messages are indented, and (for some reason) the tests want them all sorted by message.

The tests are trying to count loops at different depths, using -COUNT-<NUM> directives, e.g. 11 loops at depth 1, 40 loops at depth 2.
Alternative to sorting would be to implement support for -COUNT-<NUM>-DAG.

Sun, Dec 2, 7:46 AM

Fri, Nov 30

fedor.sergeev added inline comments to D54848: [PM] Port LoadStoreVectorizer to the new pass manager..
Fri, Nov 30, 5:40 AM
fedor.sergeev accepted D54848: [PM] Port LoadStoreVectorizer to the new pass manager..

LGTM.

Fri, Nov 30, 5:19 AM

Thu, Nov 29

fedor.sergeev added a reviewer for D54794: [NewPM] -print-before/-print-after support for new pass manager: reames.
Thu, Nov 29, 8:29 AM

Mon, Nov 26

fedor.sergeev reopened D54585: [TTI] Reduction costs only need to include a single extract element cost.

Reverted by rL347541 due to P39774 mentioned above.
It is very likely that this change is not a direct cause of the crash,
but I dont know any other ways to get rid of that crash.

Mon, Nov 26, 2:25 AM
fedor.sergeev committed rL347541: Revert "[TTI] Reduction costs only need to include a single extract element….
Revert "[TTI] Reduction costs only need to include a single extract element…
Mon, Nov 26, 2:20 AM

Sun, Nov 25

fedor.sergeev added a comment to D54585: [TTI] Reduction costs only need to include a single extract element cost.

This fix appears to cause bugs.llvm.org/show_bug.cgi?id=39774

Sun, Nov 25, 2:16 AM

Thu, Nov 22

fedor.sergeev added inline comments to D54025: [LoopSimplifyCFG] Delete dead exiting edges.
Thu, Nov 22, 12:56 AM
fedor.sergeev added inline comments to D54023: [LoopSimplifyCFG] Delete dead in-loop blocks.
Thu, Nov 22, 12:43 AM

Wed, Nov 21

fedor.sergeev planned changes to D53895: [LoopUnroll] add parsing for unroll parameters in -passes pipeline.
Wed, Nov 21, 11:10 PM
fedor.sergeev added a comment to D54794: [NewPM] -print-before/-print-after support for new pass manager.

Eventually we should implement error checking for new-pm pass names.
That will require adding more new-pm related code to the parser, and introduce some
kind of an interface with PassBuilder. Definitely *not* planned for this patch.

Wed, Nov 21, 2:12 PM
fedor.sergeev added a comment to D54794: [NewPM] -print-before/-print-after support for new pass manager.

Is the goal here anything beyond makeing -print-after/before work in the newpm?

Because if it isn't, then this change is way too complicated, and the instrumentation should just directly implement shouldPrintAfterPass, etc.

The goal is to use the same -print-after *option* to control this functionality in newpm, and not just reimplement it with a different control.

Wed, Nov 21, 2:08 PM
fedor.sergeev added a comment to D54695: [PM] Port Scalarizer to the new pass manager..

return value for Scalarizer::run fixed in rL347432

Wed, Nov 21, 2:06 PM
fedor.sergeev committed rL347432: [PM] correcting return value for new-pass-manager version of Scalarizer.
[PM] correcting return value for new-pass-manager version of Scalarizer
Wed, Nov 21, 2:04 PM
fedor.sergeev added a comment to D54695: [PM] Port Scalarizer to the new pass manager..

@bjope:
Ugh... thanks for noting this, no idea how I managed to miss that during review.
Will patch it myself.

Wed, Nov 21, 1:22 PM
fedor.sergeev updated the summary of D54794: [NewPM] -print-before/-print-after support for new pass manager.
Wed, Nov 21, 6:40 AM
fedor.sergeev added a comment to D54695: [PM] Port Scalarizer to the new pass manager..

I submitted this for markus since he doesn't have commit access yet.

Ah, sure, thanks!

Wed, Nov 21, 6:08 AM
fedor.sergeev added a reviewer for D54793: Fixing -print-module-scope for legacy SCC passes: mehdi_amini.
Wed, Nov 21, 6:05 AM
fedor.sergeev created D54794: [NewPM] -print-before/-print-after support for new pass manager.
Wed, Nov 21, 6:01 AM
fedor.sergeev created D54793: Fixing -print-module-scope for legacy SCC passes.
Wed, Nov 21, 6:00 AM
fedor.sergeev accepted D54695: [PM] Port Scalarizer to the new pass manager..

Looks ok, feel free to push. Thanks!

Wed, Nov 21, 5:20 AM
fedor.sergeev added a comment to D54695: [PM] Port Scalarizer to the new pass manager..

../lib/Transforms/Scalar/Scalarizer.cpp:311:25: error: non-const lvalue reference to type 'llvm::Function' cannot bind to a value of unrelated type 'llvm::Instruction *'

bool Done = visit(I);

You can use explicit qualification here:

bool Done = InstVisitor::visit(I);
Wed, Nov 21, 3:19 AM

Tue, Nov 20

fedor.sergeev updated the diff for D54740: [NewPM] fixing asserts on deleted loop in -print-after-all.

adding new instrumentation point to unittests

Tue, Nov 20, 8:03 AM
fedor.sergeev accepted D54695: [PM] Port Scalarizer to the new pass manager..

LGTM, but feel free to address Chandlers' note if you dont have strong feelings against it.

Tue, Nov 20, 7:23 AM
fedor.sergeev added a comment to D54695: [PM] Port Scalarizer to the new pass manager..

Looks nearly ready to go.

Tue, Nov 20, 5:47 AM
fedor.sergeev updated the diff for D54740: [NewPM] fixing asserts on deleted loop in -print-after-all.

moved "invalidated after-pass" into its own callback, looks like a proper solution, indeed.

Tue, Nov 20, 5:06 AM
fedor.sergeev added a comment to D54695: [PM] Port Scalarizer to the new pass manager..

Overall it looks really good!

Tue, Nov 20, 3:55 AM

Mon, Nov 19

fedor.sergeev added inline comments to D54740: [NewPM] fixing asserts on deleted loop in -print-after-all.
Mon, Nov 19, 11:53 PM
fedor.sergeev updated the diff for D54740: [NewPM] fixing asserts on deleted loop in -print-after-all.

scc-deleted-printer test updated

Mon, Nov 19, 11:47 PM
fedor.sergeev added a comment to D54740: [NewPM] fixing asserts on deleted loop in -print-after-all.

oops... scc-deleted-printer.ll is not yet ready - skip it until updated.
everything else should be fine.

Mon, Nov 19, 11:18 PM
fedor.sergeev created D54740: [NewPM] fixing asserts on deleted loop in -print-after-all.
Mon, Nov 19, 11:15 PM
fedor.sergeev added a comment to D50126: [Support] fix TempFile infinite loop and permission denied errors.

The test introduced here seems to be invalid.
It assumes that 16*128 attempts to choose a random hex number will cover all 16 available numbers.
Obviously, the chance for this test to fail is rather low but it is not 0.

Mon, Nov 19, 1:49 PM
fedor.sergeev committed rL347215: [LoopPass] fixing 'Modification' messages in -debug-pass=Executions for loop….
[LoopPass] fixing 'Modification' messages in -debug-pass=Executions for loop…
Mon, Nov 19, 7:14 AM
fedor.sergeev added a comment to D54695: [PM] Port Scalarizer to the new pass manager..

To me it would be a cleaner solution if you could split legacy FunctionPass interface implementation into a separate wrapper pass (e.g. ScalarizerLegacyPass),
similar to how new-pm wrapper ScalarizerPass implements all the new-pm interfaces.
That would allow to separate actual transformation (Scalarizer) from pass-manager details (ScalarizerPass/ScalarizerLegacyPass).

Mon, Nov 19, 4:56 AM

Nov 16 2018

fedor.sergeev committed rL347097: [SimpleLoopUnswitch] adding cost multiplier to cap exponential unswitch with.
[SimpleLoopUnswitch] adding cost multiplier to cap exponential unswitch with
Nov 16 2018, 1:19 PM
fedor.sergeev closed D54223: [SimpleLoopUnswitch] adding cost multiplier to cap exponential unswitch with.
Nov 16 2018, 1:19 PM

Nov 14 2018

fedor.sergeev added inline comments to D54223: [SimpleLoopUnswitch] adding cost multiplier to cap exponential unswitch with.
Nov 14 2018, 10:55 PM
fedor.sergeev updated the diff for D54223: [SimpleLoopUnswitch] adding cost multiplier to cap exponential unswitch with.

getting clones calculation more precise

Nov 14 2018, 4:25 PM
fedor.sergeev updated the diff for D54223: [SimpleLoopUnswitch] adding cost multiplier to cap exponential unswitch with.

updating names, comments etc

Nov 14 2018, 3:22 PM
fedor.sergeev added inline comments to D54223: [SimpleLoopUnswitch] adding cost multiplier to cap exponential unswitch with.
Nov 14 2018, 2:56 PM
fedor.sergeev updated the diff for D54223: [SimpleLoopUnswitch] adding cost multiplier to cap exponential unswitch with.

handle switch candidates; exponential switch case added

Nov 14 2018, 1:51 PM
fedor.sergeev planned changes to D54223: [SimpleLoopUnswitch] adding cost multiplier to cap exponential unswitch with.

Discovered a tricky testcase with 16-way switch and nested exiting branches which manages to skip the multiplier introduced here and still lead to exponential explosion.
Definitely need to check if exiting branch dominates the latch before skipping its multiplier calculation.
Also the testcase shows that we really need to go further calculating costs per candidate and using that in calculation of exponential-explosion threshold, just as Chandler asked to.

Nov 14 2018, 6:25 AM

Nov 12 2018

fedor.sergeev committed rL346740: [FileCheck] fixing docs buildbot - use proper code-block type.
[FileCheck] fixing docs buildbot - use proper code-block type
Nov 12 2018, 9:49 PM
fedor.sergeev committed rL346725: [FileCheck] fixing small formatting error in docs.
[FileCheck] fixing small formatting error in docs
Nov 12 2018, 5:15 PM
fedor.sergeev committed rL346723: [FileCheck] fixing typo in assert.
[FileCheck] fixing typo in assert
Nov 12 2018, 5:12 PM
fedor.sergeev committed rL346722: [FileCheck] introduce CHECK-COUNT-<num> repetition directive.
[FileCheck] introduce CHECK-COUNT-<num> repetition directive
Nov 12 2018, 4:49 PM
fedor.sergeev closed D54336: [FileCheck] introduce CHECK-COUNT-<num> repetition directive.
Nov 12 2018, 4:48 PM
fedor.sergeev added inline comments to D54336: [FileCheck] introduce CHECK-COUNT-<num> repetition directive.
Nov 12 2018, 2:57 PM
fedor.sergeev updated the diff for D54223: [SimpleLoopUnswitch] adding cost multiplier to cap exponential unswitch with.

renamed bottom-cap option, updated comments, tests

Nov 12 2018, 12:59 PM
fedor.sergeev added inline comments to D54223: [SimpleLoopUnswitch] adding cost multiplier to cap exponential unswitch with.
Nov 12 2018, 12:48 PM
fedor.sergeev updated the diff for D54223: [SimpleLoopUnswitch] adding cost multiplier to cap exponential unswitch with.

more extensive checking for exiting branches; adding tests with exiting branches

Nov 12 2018, 12:43 PM
fedor.sergeev updated the diff for D54223: [SimpleLoopUnswitch] adding cost multiplier to cap exponential unswitch with.

update as per Max' comments

Nov 12 2018, 3:40 AM
fedor.sergeev added inline comments to D54223: [SimpleLoopUnswitch] adding cost multiplier to cap exponential unswitch with.
Nov 12 2018, 3:39 AM

Nov 11 2018

fedor.sergeev added a parent revision for D54223: [SimpleLoopUnswitch] adding cost multiplier to cap exponential unswitch with: D54336: [FileCheck] introduce CHECK-COUNT-<num> repetition directive.
Nov 11 2018, 4:11 AM
fedor.sergeev added a child revision for D54336: [FileCheck] introduce CHECK-COUNT-<num> repetition directive: D54223: [SimpleLoopUnswitch] adding cost multiplier to cap exponential unswitch with.
Nov 11 2018, 4:11 AM

Nov 10 2018

fedor.sergeev added a comment to D54337: [ASan] Make AddressSanitizer a ModulePass.

The problem is not speed but correctness. The initialization adds function definitions, which function passes can't do.

Hmm... can you point me where function definitions are being added in doInitialization, I just dont see it (frankly I'm not familiar with this code at all).

Nov 10 2018, 3:51 AM · Restricted Project
fedor.sergeev added a comment to D54374: [newpm] Add an OptimizerLast EP.

A key difference between this and legacy is at O0, this EP won't be triggered.

Makes sense to mention this in commit message...

Nov 10 2018, 2:56 AM
fedor.sergeev added a comment to D54337: [ASan] Make AddressSanitizer a ModulePass.

Making this a module pass will cost us all the nice chaining and cache locality we get from a function pass.
That will make the already slow instrumentation even more expensive. Do you see a way to keep this as a function pass?

I assume the main problem with this being function pass in NewPM is the need for initialization, which is too slow when being done per-function.

Nov 10 2018, 2:55 AM · Restricted Project
fedor.sergeev accepted D54374: [newpm] Add an OptimizerLast EP.
Nov 10 2018, 2:07 AM
fedor.sergeev added a comment to D54374: [newpm] Add an OptimizerLast EP.

LGTM (though I have no idea what legacy does for this :) )

Nov 10 2018, 2:07 AM
fedor.sergeev updated the diff for D54336: [FileCheck] introduce CHECK-COUNT-<num> repetition directive.

minor cleanup

Nov 10 2018, 2:01 AM
fedor.sergeev added inline comments to D54336: [FileCheck] introduce CHECK-COUNT-<num> repetition directive.
Nov 10 2018, 12:49 AM

Nov 9 2018

fedor.sergeev updated the diff for D54336: [FileCheck] introduce CHECK-COUNT-<num> repetition directive.

addings docs

Nov 9 2018, 1:08 PM
fedor.sergeev updated the summary of D54336: [FileCheck] introduce CHECK-COUNT-<num> repetition directive.
Nov 9 2018, 12:41 PM
fedor.sergeev updated the diff for D54336: [FileCheck] introduce CHECK-COUNT-<num> repetition directive.

addressing comments, clang-formatted, added std includes, some minor stuff

Nov 9 2018, 12:35 PM
fedor.sergeev added a comment to D54336: [FileCheck] introduce CHECK-COUNT-<num> repetition directive.

I know, it needs documentation changes :)

Nov 9 2018, 11:19 AM
fedor.sergeev added reviewers for D54336: [FileCheck] introduce CHECK-COUNT-<num> repetition directive: probinson, bkramer, dblaikie.
Nov 9 2018, 11:18 AM
fedor.sergeev created D54336: [FileCheck] introduce CHECK-COUNT-<num> repetition directive.
Nov 9 2018, 11:17 AM