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 (105 w, 5 d)

Recent Activity

Today

fedor.sergeev added a comment to D58406: Fix IR/Analysis layering issue in OptBisect.

Keep in mind that for the new PM we're talking about an API that's much more powerful, because it's designed for arbitrary hooks.
OptBisect doesn't need that necessarily. I'm in favor of passing the strings instead.

Yep, thats true.
Was just pointing out that new PM might benefit from generalization, if anybody decides to do it ;)

Wed, Feb 20, 10:08 AM
fedor.sergeev added a reviewer for D58406: Fix IR/Analysis layering issue in OptBisect: chandlerc.
Wed, Feb 20, 8:20 AM
fedor.sergeev added a comment to D58406: Fix IR/Analysis layering issue in OptBisect.

Alternatively - how would you feel about callers passing in the strings directly?

return !BisectEnabled || checkPass(P->getPassName(), P->getDescription(U));

The Pass and Unit are only used to retrieve the Pass Name and Description (passing the Unit straight back to the Pass that just passed it in) - so why not pass in those (Pass Name and Description) instead?

You mean e.g. changing

!F.getContext().getOptPassGate().shouldRunPass(this, &F))

into

!F.getContext().getOptPassGate().shouldRunPass(this->getName(), F.getDescription()))
Wed, Feb 20, 8:19 AM
fedor.sergeev added a comment to D58406: Fix IR/Analysis layering issue in OptBisect.

The need for generic way of getting textual description for IRUnit is obvious both in legacy and new pass manager.
However in this particular approach I dont like how "const void*" is used to erase specific type of IRUnit.

Wed, Feb 20, 7:48 AM
fedor.sergeev added a reviewer for D58406: Fix IR/Analysis layering issue in OptBisect: philip.pfaffe.
Wed, Feb 20, 7:25 AM

Yesterday

fedor.sergeev added a comment to D58231: [LICM] Support of widenable condition guards in LICM.

The main motivation is unification - I don't want to leave a single thing that is supported for intrinsic guards and not supported for control flow guards. Unswitching has known problems related to exponential code size growth, so I don't want our ability to eliminate guards in loops to depend on its cost model.

Trivial unswitch does not have a problem of exponential code growth.
Basically, what you do here does match what trivial unswitch does.
So in order to rely on unswitch we just need to ensure that this widenable branch thing does fall into a trivial unswitch category.

Tue, Feb 19, 8:10 AM · Restricted Project

Fri, Feb 15

fedor.sergeev added a comment to D58231: [LICM] Support of widenable condition guards in LICM.

LGTM, but I would like to have somebody else to take a look at this as well...

Fri, Feb 15, 3:41 AM · Restricted Project

Tue, Feb 12

fedor.sergeev added a comment to D57231: [LoopSimplifyCFG] Pay respect to LCSSA when removing dead blocks.

I'll split out changes in basic block utils as separate NFC, then submit change in LoopSimplifyCFG separately.

Thanks, your effort is really appreciated!

Tue, Feb 12, 12:30 AM · Restricted Project

Mon, Feb 11

fedor.sergeev added a comment to D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting.
In D57265#1393453, @vsk wrote:

I'd prefer not adding this kind of state to PassBuilder. SplitColdCode is soemthing that refers to the construction of one particular pipeline, not to pipeline-building in general. It should be an argument passed down through the build*Pipeline calls.

I'm not sure I understand the distinction being made here -- could you elaborate? Isn't enabling a pass related to pipeline-building in general? If not, I don't see how arguments to build*Pipeline do become related to pipeline-building in general.

For context, I've modeled the addition of the SplitColdCode member to PassBuilder on PGOOpt (c.f. PGOOptions::RunProfileGen). One thing I like about this approach is that it doesn't require changing IR, or changing very many different APIs. But if it's really not reasonable, I'm happy to take another shot at it.

I cant say for PGOOpt in particular, but overall idea of PassBuilder is that, well, it is not "builder" as per "builder pattern".
You do not fill it with parts of a complex pipeline object and then press a magical "build" button.
Instead you ask it to build various "named" pipelines, or just parse it from text.
If you compare with PassManagerBuilder, which contains a dozen of various data members that seem to drive the pipeline construction,
PassBuilder contains only a few. And the purpose of PassBuilder members is to be used during individual *pass*es creation.

Mon, Feb 11, 2:40 PM · Restricted Project
fedor.sergeev accepted D57231: [LoopSimplifyCFG] Pay respect to LCSSA when removing dead blocks.

LGTM, but please, schedule those followups discussed here.

Mon, Feb 11, 5:33 AM · Restricted Project
fedor.sergeev accepted D57221: [LoopSimplifyCFG] Change logic of dead loops removal to avoid hitting asserts.

LGTM.

Mon, Feb 11, 5:30 AM · Restricted Project

Wed, Feb 6

fedor.sergeev added a comment to D57793: [NewPM][MSan] Add sanitizer at O0.

Please, update the msan test that currently runs with -O1.

Wed, Feb 6, 6:41 AM · Restricted Project

Mon, Feb 4

fedor.sergeev accepted D57640: [NewPM][MSan] Add Options Handling.

LGTM.

Mon, Feb 4, 12:59 PM · Restricted Project, Restricted Project
fedor.sergeev accepted D56550: Add recipes for migrating downstream branches of git mirrors.

So, what are the plans with this?
I'm happy with it as is.

Mon, Feb 4, 2:17 AM · Restricted Project
fedor.sergeev accepted D56470: [NewPM] Second attempt at porting ASan.

LGTM bar the small nit below.

Mon, Feb 4, 12:46 AM · Restricted Project, Restricted Project
fedor.sergeev added inline comments to D57640: [NewPM][MSan] Add Options Handling.
Mon, Feb 4, 12:12 AM · Restricted Project, Restricted Project

Tue, Jan 22

fedor.sergeev accepted D57018: Fix lvm::is_trivially_copyable portability issues.

Both the small reproducer and full LLVM-only builds/tests passed with my gcc 4.9.
LGTM

Tue, Jan 22, 12:59 AM

Mon, Jan 21

fedor.sergeev added a comment to D57018: Fix lvm::is_trivially_copyable portability issues.

Started full build, will report as soon as it is ready.

On xray-converter.cpp fails the same way as on my shorter reproducer.

Mon, Jan 21, 3:19 PM
fedor.sergeev added a comment to D57018: Fix lvm::is_trivially_copyable portability issues.

it does *not* work on my shorter reproducer:

] g++ -std=c++11 -I ../../llvm/include -I include ../../llvm/copyable_with_llvm.cpp 
../../llvm/copyable_with_llvm.cpp: In instantiation of ‘struct TrieNode<StackIdData>’:
../../llvm/include/llvm/Support/type_traits.h:118:34:   required by substitution of ‘template<class T> using IsCopyAssignableImpl = decltype ((declval<T&>)()=(declval<const T&>)()) [with T = llvm::detail::trivial_helper<TrieNode<StackIdData>*>]’
../../llvm/include/llvm/Support/type_traits.h:114:8:   required from ‘struct llvm::detail::IsDetected<llvm::detail::IsCopyAssignableImpl, llvm::detail::trivial_helper<TrieNode<StackIdData>*> >’
../../llvm/include/llvm/Support/type_traits.h:149:8:   required from ‘struct llvm::is_copy_assignable<llvm::detail::trivial_helper<TrieNode<StackIdData>*> >’
../../llvm/include/llvm/Support/type_traits.h:174:25:   required from ‘constexpr const bool llvm::is_trivially_copyable<TrieNode<StackIdData>*>::has_trivial_copy_assign’
../../llvm/include/llvm/Support/type_traits.h:195:32:   required from ‘constexpr const bool llvm::is_trivially_copyable<TrieNode<StackIdData>*>::value’
../../llvm/copyable_with_llvm.cpp:12:64:   required from here
../../llvm/copyable_with_llvm.cpp:7:18: error: ‘TrieNode<AssociatedData>::ExtraData’ has incomplete type
   AssociatedData ExtraData;
                  ^
../../llvm/copyable_with_llvm.cpp:10:8: error: forward declaration of ‘struct StackIdData’
 struct StackIdData {
        ^

Started full build, will report as soon as it is ready.

Mon, Jan 21, 2:53 PM
fedor.sergeev added inline comments to D57018: Fix lvm::is_trivially_copyable portability issues.
Mon, Jan 21, 1:29 PM
fedor.sergeev requested changes to D57018: Fix lvm::is_trivially_copyable portability issues.

Sorry, now it fails on is_move_assignable:

] cat copyable_with_llvm.cpp
#include "llvm/Support/type_traits.h"
// code from tools/llvm-xray/xray-converter.cpp
template <typename AssociatedData> struct TrieNode {
  int FuncId;
  AssociatedData ExtraData;
};
struct StackIdData {
  // to get is_trivially_copyable instantiation from within the class
  int x [llvm::is_trivially_copyable<TrieNode<StackIdData> *>::value + 1];
};
] g++ -std=c++11 -I ../../llvm/include -I include copyable_with_llvm.cpp 
copyable_with_llvm.cpp: In instantiation of ‘struct TrieNode<StackIdData>’:
/gcc/include/c++/4.9.2/type_traits:1183:45:   required by substitution of ‘template<class _Tp1, class _Up1, class> static std::true_type std::__is_assignable_helper<_Tp, _Up>::__test(int) [with _Tp1 = llvm::detail::trivial_helper<TrieNode<StackIdData>*>&; _Up1 = llvm::detail::trivial_helper<TrieNode<StackIdData>*>&&; <template-parameter-1-3> = <missing>]’
/gcc/include/c++/4.9.2/type_traits:1192:30:   required from ‘class std::__is_assignable_helper<llvm::detail::trivial_helper<TrieNode<StackIdData>*>&, llvm::detail::trivial_helper<TrieNode<StackIdData>*>&&>’
/gcc/include/c++/4.9.2/type_traits:1197:12:   required from ‘struct std::is_assignable<llvm::detail::trivial_helper<TrieNode<StackIdData>*>&, llvm::detail::trivial_helper<TrieNode<StackIdData>*>&&>’
/gcc/include/c++/4.9.2/type_traits:1227:12:   required from ‘struct std::__is_move_assignable_impl<llvm::detail::trivial_helper<TrieNode<StackIdData>*>, true>’
/gcc/include/c++/4.9.2/type_traits:1233:12:   required from ‘struct std::is_move_assignable<llvm::detail::trivial_helper<TrieNode<StackIdData>*> >’
../../llvm/include/llvm/Support/type_traits.h:157:25:   required from ‘constexpr const bool llvm::is_trivially_copyable<TrieNode<StackIdData>*>::has_trivial_move_assign’
../../llvm/include/llvm/Support/type_traits.h:170:32:   required from ‘constexpr const bool llvm::is_trivially_copyable<TrieNode<StackIdData>*>::value’
copyable_with_llvm.cpp:9:64:   required from here
copyable_with_llvm.cpp:5:18: error: ‘TrieNode<AssociatedData>::ExtraData’ has incomplete type
   AssociatedData ExtraData;
                  ^
copyable_with_llvm.cpp:7:8: error: forward declaration of ‘struct StackIdData’
 struct StackIdData {
        ^

And yes, the same failure on the actual llvm build on xray-converter.cpp.

Mon, Jan 21, 12:42 PM

Jan 21 2019

fedor.sergeev added a comment to D54472: Disable invalid isPodLike<> specialization.

And the smallest testcase I'v been able to come up with:

Jan 21 2019, 3:56 AM
fedor.sergeev added a comment to D54472: Disable invalid isPodLike<> specialization.

I'm sorry to bring bad news, but this fails for our gcc 4.9.2 build, when building llvm/tools/llvm-xray/xray-converter.cpp.
Failures looks like this:

Jan 21 2019, 3:20 AM

Jan 18 2019

fedor.sergeev added inline comments to D56550: Add recipes for migrating downstream branches of git mirrors.
Jan 18 2019, 10:03 AM · Restricted Project
fedor.sergeev added a comment to D56550: Add recipes for migrating downstream branches of git mirrors.

Is there anything substantial left here?
Since we already have a blessed repo it makes sense to push this, even if it is not 200% ready.

Jan 18 2019, 9:56 AM · Restricted Project

Jan 16 2019

fedor.sergeev accepted D56783: [SLP] Fix PR40310: The reduction nodes should stay scalar..

Looks good.
Thanks for a taking care of this!

Jan 16 2019, 7:04 AM
fedor.sergeev abandoned D56714: [SLP] introduce control over arithmetic horizontal reduction.

There is a proper fix suggested by @ABataev in PR40310, dropping this.

Jan 16 2019, 6:24 AM
fedor.sergeev accepted D56121: [LoopSimplifyCFG] Fix order of deletion of complex dead subloops.

LGTM.

Jan 16 2019, 4:50 AM
fedor.sergeev accepted D56144: [LoopSimplifyCFG] Form LCSSA when a parent loop becomes a sibling.

LGTM.

Jan 16 2019, 3:26 AM

Jan 15 2019

fedor.sergeev added a comment to D56714: [SLP] introduce control over arithmetic horizontal reduction.

I'm really not sure about this - PR40310 is most likely a bug in the reduction code so isn't it likely that Add/FAdd can be affected by it as well?

I'm not pushing this particular solution, its here only as a possible short-term.
However, I need to note that we have fuzzer running continuously and it started regularly finding bugs in reduction only after the aforementioned change went in.
Since late-October time we found ~10 different variations of it.

Jan 15 2019, 7:17 AM
fedor.sergeev updated the diff for D56714: [SLP] introduce control over arithmetic horizontal reduction.

adding control to a couple tests that expect hor to happen

Jan 15 2019, 3:31 AM
fedor.sergeev updated the diff for D56714: [SLP] introduce control over arithmetic horizontal reduction.

clang-formatting

Jan 15 2019, 3:17 AM
fedor.sergeev created D56714: [SLP] introduce control over arithmetic horizontal reduction.
Jan 15 2019, 3:10 AM

Jan 11 2019

fedor.sergeev abandoned D53895: [LoopUnroll] add parsing for unroll parameters in -passes pipeline.

This has been pushed as https://reviews.llvm.org/rL350808, just neglected to put a reference to this review, so it did not auto-close.

Jan 11 2019, 5:11 AM
fedor.sergeev removed a reviewer for D53895: [LoopUnroll] add parsing for unroll parameters in -passes pipeline: fedor.sergeev.
Jan 11 2019, 2:35 AM
fedor.sergeev accepted D53895: [LoopUnroll] add parsing for unroll parameters in -passes pipeline.
Jan 11 2019, 2:35 AM
fedor.sergeev added 1 commit(s) for D53895: [LoopUnroll] add parsing for unroll parameters in -passes pipeline: rL350808: [LoopUnroll] add parsing for unroll parameters in -passes pipeline.
Jan 11 2019, 2:34 AM
fedor.sergeev added an edge to rL350808: [LoopUnroll] add parsing for unroll parameters in -passes pipeline: D53895: [LoopUnroll] add parsing for unroll parameters in -passes pipeline.
Jan 11 2019, 2:34 AM

Jan 10 2019

fedor.sergeev added a comment to D56550: Add recipes for migrating downstream branches of git mirrors.

Thanks for the effort!

Jan 10 2019, 12:31 PM · Restricted Project
fedor.sergeev added inline comments to D56550: Add recipes for migrating downstream branches of git mirrors.
Jan 10 2019, 11:46 AM · Restricted Project
fedor.sergeev added a comment to D56538: [NewPM][TSan] Reiterate the TSan port.

Just a side note - seeing a problem with dyld wrt original tsan_init solution, do we have the same problem with msan_init, which was similarly solved in D55647?

Jan 10 2019, 4:49 AM
fedor.sergeev committed rL350808: [LoopUnroll] add parsing for unroll parameters in -passes pipeline.
[LoopUnroll] add parsing for unroll parameters in -passes pipeline
Jan 10 2019, 2:07 AM

Jan 9 2019

fedor.sergeev added a comment to D53895: [LoopUnroll] add parsing for unroll parameters in -passes pipeline.

ping?

Jan 9 2019, 3:18 PM

Jan 6 2019

fedor.sergeev updated the diff for D53895: [LoopUnroll] add parsing for unroll parameters in -passes pipeline.

cleanup

Jan 6 2019, 10:41 AM

Jan 3 2019

fedor.sergeev updated the diff for D53895: [LoopUnroll] add parsing for unroll parameters in -passes pipeline.

okey, now without explicit ParametersT template parameter.

Jan 3 2019, 1:15 PM
fedor.sergeev added inline comments to D53895: [LoopUnroll] add parsing for unroll parameters in -passes pipeline.
Jan 3 2019, 12:51 PM
fedor.sergeev added inline comments to D53895: [LoopUnroll] add parsing for unroll parameters in -passes pipeline.
Jan 3 2019, 7:41 AM
fedor.sergeev added a comment to D53895: [LoopUnroll] add parsing for unroll parameters in -passes pipeline.

ping?

Jan 3 2019, 2:44 AM

Dec 24 2018

fedor.sergeev added inline comments to D53895: [LoopUnroll] add parsing for unroll parameters in -passes pipeline.
Dec 24 2018, 5:14 AM
fedor.sergeev updated the diff for D53895: [LoopUnroll] add parsing for unroll parameters in -passes pipeline.

addressing comments

Dec 24 2018, 5:09 AM
fedor.sergeev added a comment to D55605: [NewPM] Port Msan: Alternative approach using a module to create the global constructor.

Abandoning in favor of D55605.

I bet you meant D55647 here :)

Dec 24 2018, 1:46 AM

Dec 21 2018

fedor.sergeev updated the diff for D53895: [LoopUnroll] add parsing for unroll parameters in -passes pipeline.

finally, moved parameters parsing from LoopUnrollPass into PassBuilder,
addressing @chandlerc's comments

Dec 21 2018, 1:10 PM
fedor.sergeev accepted D55647: [NewPM] Port Msan: Second alternative approach without any module level initalization.

I'm not very faimiliar with details of llvm.global_ctors, but solution makes sense.
Code looks good bar minor access nits.
Thus, LGTM.

Dec 21 2018, 4:59 AM
fedor.sergeev committed rL349896: [NewPM] -print-module-scope -print-after now prints module even after….
[NewPM] -print-module-scope -print-after now prints module even after…
Dec 21 2018, 3:52 AM
fedor.sergeev closed D55278: [NewPM] -print-module-scope -print-after now prints module even after invalidated Loop/SCC.
Dec 21 2018, 3:52 AM

Dec 20 2018

fedor.sergeev added inline comments to D55278: [NewPM] -print-module-scope -print-after now prints module even after invalidated Loop/SCC.
Dec 20 2018, 5:36 AM
fedor.sergeev updated the diff for D55278: [NewPM] -print-module-scope -print-after now prints module even after invalidated Loop/SCC.

fixing Optional use

Dec 20 2018, 5:31 AM
fedor.sergeev updated the diff for D55278: [NewPM] -print-module-scope -print-after now prints module even after invalidated Loop/SCC.

addressing comments

Dec 20 2018, 3:58 AM

Dec 12 2018

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

Dec 11 2018

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

ping?

Dec 11 2018, 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
Dec 11 2018, 11:09 AM
fedor.sergeev closed D54740: [NewPM] fixing asserts on deleted loop in -print-after-all.
Dec 11 2018, 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...

Dec 11 2018, 4:10 AM

Dec 10 2018

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.

Dec 10 2018, 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.

Dec 10 2018, 1:26 PM

Dec 7 2018

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

removing unneeded asserts

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

LGTM.

Dec 7 2018, 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...

Dec 7 2018, 4:48 AM

Dec 6 2018

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.

Dec 6 2018, 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.
Dec 6 2018, 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

Dec 6 2018, 3:14 AM

Dec 5 2018

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?

Dec 5 2018, 6:12 AM

Dec 4 2018

fedor.sergeev added a reviewer for D51207: Introduce llvm.experimental.widenable_condition intrinsic: chandlerc.
Dec 4 2018, 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)

Dec 4 2018, 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

Dec 4 2018, 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.
Dec 4 2018, 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.
Dec 4 2018, 8:49 AM
fedor.sergeev created D55278: [NewPM] -print-module-scope -print-after now prints module even after invalidated Loop/SCC.
Dec 4 2018, 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

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

Dec 3 2018

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.

Dec 3 2018, 1:06 PM · Restricted Project, 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.

Dec 3 2018, 12:47 PM · Restricted Project, Restricted Project
fedor.sergeev committed rL348144: Fixing -print-module-scope for legacy SCC passes.
Fixing -print-module-scope for legacy SCC passes
Dec 3 2018, 6:51 AM
fedor.sergeev closed D54793: Fixing -print-module-scope for legacy SCC passes.
Dec 3 2018, 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.

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

Dec 2 2018

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

ping?

Dec 2 2018, 1:26 PM
fedor.sergeev accepted D55168: [test] Fix use of 'sort -b' in SimpleLoopUnswitch on NetBSD.
Dec 2 2018, 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.

Dec 2 2018, 7:46 AM

Nov 30 2018

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

LGTM.

Nov 30 2018, 5:19 AM

Nov 29 2018

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

Nov 26 2018

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.

Nov 26 2018, 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…
Nov 26 2018, 2:20 AM

Nov 25 2018

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

Nov 25 2018, 2:16 AM

Nov 22 2018

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

Nov 21 2018

fedor.sergeev planned changes to D53895: [LoopUnroll] add parsing for unroll parameters in -passes pipeline.
Nov 21 2018, 11:10 PM