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 (114 w, 22 h)

Recent Activity

Thu, Apr 18

fedor.sergeev added inline comments to D60676: [NewPM] Add Option handling for SimpleLoopUnswitch.
Thu, Apr 18, 10:54 PM
fedor.sergeev added a comment to D60814: [PassBuilder] promote pass-pipeline parsing API to public.

see D60870 for a proof-of-concept implementation of PipelineParser.

Thu, Apr 18, 2:05 PM · Restricted Project
fedor.sergeev added a comment to D60870: POC. [PassBuilder] introduce a separate pass-pipeline parsing API.

This is not yet complete (in particular parseAA is still in PassBuilder atm).
However I would like to get a feedback on overall organization.
Right now:

  • PipelineParser is a nested class in PassBuilder
  • it is created for a single text-pipeline-parsing query
  • text is processed into a pipeline vector and kept in the parser
  • callbacks still reside in pass builder, accessed through a reference to pass builder kept in the parser
  • callbacks take both Parser and array of Elements
Thu, Apr 18, 8:24 AM · Restricted Project
fedor.sergeev created D60870: POC. [PassBuilder] introduce a separate pass-pipeline parsing API.
Thu, Apr 18, 7:50 AM · Restricted Project

Wed, Apr 17

fedor.sergeev accepted D60681: [NewPM] Add Option handling for LoopVectorize.

LGTM.

Wed, Apr 17, 10:42 PM · Restricted Project
fedor.sergeev requested changes to D60676: [NewPM] Add Option handling for SimpleLoopUnswitch.
Wed, Apr 17, 10:42 PM
fedor.sergeev added a comment to D60814: [PassBuilder] promote pass-pipeline parsing API to public.

Yeah, the array-ref API here is already pretty leaky. It'd be really nice if we could only give the callbacks the necessary APIs here, but not sure its worth building that abstraction. I think the way I'd do it is create a parser type that is only built by the PassBuilder and is passed into the callbacks. It would then have these APIs, etc. It'd also be used internally to implement the main parsing API. Thoughts?

It might be a bit conceptually cleaner, but other than renaming the object from PassBuilder to PassPipelineParser (and internally talking to PassBuilder), what would be the API difference
compared to what is being suggested here?
I see a bunch of questions, e.g. - should we keep the callbacks in the parser object? etc
And I dont really see what practical benefits would we get from separating the parsing API out...

Wed, Apr 17, 8:05 AM · Restricted Project
fedor.sergeev added a comment to D60814: [PassBuilder] promote pass-pipeline parsing API to public.

What is your specific usecase for this? You have a custom PassManager which can contain, e.g., a function pass pipeline?

Right. We are building our own "kinda-SCC" pass manager, which runs inlining-devirtualization iteration doing walk over functions in rather different order than current SCC does.
This pass manager right now accepts at least one function pass pipeline (and we think about adding one more), and I would like to be able to populate this pass manager from -passes.

Wed, Apr 17, 3:33 AM · Restricted Project

Tue, Apr 16

fedor.sergeev added a comment to D60783: [LoopPred] Implement a version of the profitability heuristic w/o BPI.

In the new pass manager, getting access to BPI reliability is a challenge. In practice, we've been running downstream with a variation of this non-BPI based heuristic.

Can you please explain why? The new pass manager was supposed to make our problems getting a hold of analysis results in different places easier, not harder.

New pass manager provides proper organization of analyses but it cant (and shouldnt!) hide innate asymmetry in relationship between Function-level analysis and Loop-level transformation.
NPM specifically prohibits reruns of outer-level analysis from within a inner-level-pass (proxy is readonly).
When working on loop-level you can only *use* function/module-level analysis, manually *preserve* it or automatically invalidate analysis results as a whole after the changes to IR.
That seems to be a proper design decision.

Tue, Apr 16, 11:52 PM · Restricted Project
fedor.sergeev created D60814: [PassBuilder] promote pass-pipeline parsing API to public.
Tue, Apr 16, 11:21 PM · Restricted Project
fedor.sergeev updated the summary of D60740: [InlineCost] cleanup calculations of Cost and Threshold.
Tue, Apr 16, 3:13 AM · Restricted Project
fedor.sergeev updated the summary of D60740: [InlineCost] cleanup calculations of Cost and Threshold.
Tue, Apr 16, 3:13 AM · Restricted Project
fedor.sergeev added a reviewer for D60751: [NFC][InlineCost] cleanup - comments, overflow handling.: apilipenko.
Tue, Apr 16, 3:08 AM · Restricted Project
fedor.sergeev added a reviewer for D60740: [InlineCost] cleanup calculations of Cost and Threshold: apilipenko.
Tue, Apr 16, 3:07 AM · Restricted Project

Mon, Apr 15

fedor.sergeev updated the summary of D60740: [InlineCost] cleanup calculations of Cost and Threshold.
Mon, Apr 15, 10:40 PM · Restricted Project
fedor.sergeev updated the summary of D60740: [InlineCost] cleanup calculations of Cost and Threshold.
Mon, Apr 15, 10:40 PM · Restricted Project
fedor.sergeev added a child revision for D60751: [NFC][InlineCost] cleanup - comments, overflow handling.: D60740: [InlineCost] cleanup calculations of Cost and Threshold.
Mon, Apr 15, 10:33 PM · Restricted Project
fedor.sergeev added a parent revision for D60740: [InlineCost] cleanup calculations of Cost and Threshold: D60751: [NFC][InlineCost] cleanup - comments, overflow handling..
Mon, Apr 15, 10:33 PM · Restricted Project
fedor.sergeev created D60751: [NFC][InlineCost] cleanup - comments, overflow handling..
Mon, Apr 15, 10:33 PM · Restricted Project
fedor.sergeev updated the diff for D60740: [InlineCost] cleanup calculations of Cost and Threshold.

moving NFC part out of this change

Mon, Apr 15, 10:32 PM · Restricted Project
fedor.sergeev added a child revision for D60412: [CallSite removal] Move the legacy PM, call graph, and some inliner code to `CallBase`.: D60636: [CallSite removal] move InlineCost to CallBase usage.
Mon, Apr 15, 9:53 PM · Restricted Project
fedor.sergeev added a parent revision for D60636: [CallSite removal] move InlineCost to CallBase usage: D60412: [CallSite removal] Move the legacy PM, call graph, and some inliner code to `CallBase`..
Mon, Apr 15, 9:53 PM · Restricted Project
fedor.sergeev added inline comments to D60740: [InlineCost] cleanup calculations of Cost and Threshold.
Mon, Apr 15, 9:45 PM · Restricted Project
fedor.sergeev created D60740: [InlineCost] cleanup calculations of Cost and Threshold.
Mon, Apr 15, 5:16 PM · Restricted Project
fedor.sergeev added a reviewer for D59918: [Attributor] Pass infrastructure and fixpoint framework: chandlerc.
Mon, Apr 15, 3:00 PM · Restricted Project

Sun, Apr 14

fedor.sergeev accepted D60675: [NewPM] Add Option handling for SimplifyCFG.

LGTM.

Sun, Apr 14, 11:30 PM · Restricted Project
fedor.sergeev added inline comments to D60675: [NewPM] Add Option handling for SimplifyCFG.
Sun, Apr 14, 11:02 PM · Restricted Project

Fri, Apr 12

fedor.sergeev created D60636: [CallSite removal] move InlineCost to CallBase usage.
Fri, Apr 12, 3:00 PM · Restricted Project

Tue, Apr 9

fedor.sergeev added a reviewer for D59920: [FunctionAttrs] Remove old "returned" argument deduction: chandlerc.
Tue, Apr 9, 2:03 AM · Restricted Project
fedor.sergeev added a reviewer for D59919: [Attributor] Deduce "returned" argument attribute: chandlerc.
Tue, Apr 9, 2:03 AM · Restricted Project, Restricted Project

Mon, Apr 8

fedor.sergeev accepted D60412: [CallSite removal] Move the legacy PM, call graph, and some inliner code to `CallBase`..

LGTM, thanks!

Mon, Apr 8, 9:07 AM · Restricted Project

Mon, Apr 1

fedor.sergeev added a comment to D59825: SafepointIRVerifier port to new Pass Manager.

Landed the change back, with fixes to module.modulemap. r357361.

Mon, Apr 1, 3:43 AM · Restricted Project

Sun, Mar 31

fedor.sergeev committed rGa2ed448bf24f: SafepointIRVerifier port to new Pass Manager (authored by fedor.sergeev).
SafepointIRVerifier port to new Pass Manager
Sun, Mar 31, 3:15 AM
fedor.sergeev committed rL357361: SafepointIRVerifier port to new Pass Manager.
SafepointIRVerifier port to new Pass Manager
Sun, Mar 31, 3:14 AM

Thu, Mar 28

fedor.sergeev requested changes to D57835: Fix -ftime-report with -x ir.

just to make the comments visible :)

Thu, Mar 28, 4:47 AM
fedor.sergeev added a comment to D59869: [NewPM] Fix a nasty bug with analysis invalidation in the new PM..

Pushed this through our java-based fuzzer, no problems found so far
(though our use of SCC-based passes is very limited).

Thu, Mar 28, 1:08 AM · Restricted Project

Tue, Mar 26

fedor.sergeev accepted D59825: SafepointIRVerifier port to new Pass Manager.

LGTM

Tue, Mar 26, 11:11 PM · Restricted Project
fedor.sergeev added inline comments to D59825: SafepointIRVerifier port to new Pass Manager.
Tue, Mar 26, 11:06 PM · Restricted Project
fedor.sergeev added a comment to D59825: SafepointIRVerifier port to new Pass Manager.

Please, update verify-safepoint tests to use new-pass-manager flavor.

Tue, Mar 26, 11:16 AM · Restricted Project

Fri, Mar 22

fedor.sergeev committed rGec74378e93fe: [Legacy][TimePasses] allow -time-passes reporting into a custom stream (authored by fedor.sergeev).
[Legacy][TimePasses] allow -time-passes reporting into a custom stream
Fri, Mar 22, 4:11 PM
fedor.sergeev committed rL356824: [Legacy][TimePasses] allow -time-passes reporting into a custom stream.
[Legacy][TimePasses] allow -time-passes reporting into a custom stream
Fri, Mar 22, 4:10 PM
fedor.sergeev closed D59416: [Legacy][TimePasses] allow -time-passes reporting into a custom stream.
Fri, Mar 22, 4:10 PM · Restricted Project

Mar 20 2019

fedor.sergeev updated the diff for D59416: [Legacy][TimePasses] allow -time-passes reporting into a custom stream.

comments updated

Mar 20 2019, 2:45 PM · Restricted Project
fedor.sergeev added inline comments to D59416: [Legacy][TimePasses] allow -time-passes reporting into a custom stream.
Mar 20 2019, 1:20 PM · Restricted Project
fedor.sergeev added inline comments to D59416: [Legacy][TimePasses] allow -time-passes reporting into a custom stream.
Mar 20 2019, 5:34 AM · Restricted Project
fedor.sergeev added inline comments to D59416: [Legacy][TimePasses] allow -time-passes reporting into a custom stream.
Mar 20 2019, 5:23 AM · Restricted Project
fedor.sergeev updated the diff for D59416: [Legacy][TimePasses] allow -time-passes reporting into a custom stream.

moved legacy test code around to remove the need for additional namespace manipulations

Mar 20 2019, 4:42 AM · Restricted Project

Mar 17 2019

fedor.sergeev planned changes to D51632: [New PM][PassInstrumentation] enhancing PassInstrumentation with PassManager tracking.

changing status to reflect that there is no active work on this patch atm.

Mar 17 2019, 11:16 PM · Restricted Project

Mar 15 2019

fedor.sergeev added inline comments to D57835: Fix -ftime-report with -x ir.
Mar 15 2019, 3:27 PM
fedor.sergeev added a comment to D57835: Fix -ftime-report with -x ir.

I partially solved the problem with double printing the reports. It's from this in cc1_main:

// If any timers were active but haven't been destroyed yet, print theirp
// results now.  This happens in -disable-free mode.
llvm::TimerGroup::printAll(llvm::errs());

On a source file, -ftime-report -disable-free prints one report. -ftime-report without disable-free prints 2. For the IR file, both happen either way and it double prints. The printing destructor can still be called from the ManagedStatic in PassTimingInfo

I dont quite understand what clang is doing in terms of timers (and what -disable-free is/how does it interact with -ftime-report).
However if after doing printAll() you expect that there wont be any other reports then you should call clearAll() immediately after.

Mar 15 2019, 3:24 PM
fedor.sergeev committed rG6a9c2f4f9881: [TimePasses] allow -time-passes reporting into a custom stream (authored by fedor.sergeev).
[TimePasses] allow -time-passes reporting into a custom stream
Mar 15 2019, 3:16 PM
fedor.sergeev committed rL356305: [TimePasses] allow -time-passes reporting into a custom stream.
[TimePasses] allow -time-passes reporting into a custom stream
Mar 15 2019, 3:15 PM
fedor.sergeev closed D59366: [NewPM][TimePasses] allow -time-passes reporting into a custom stream.
Mar 15 2019, 3:15 PM · Restricted Project
fedor.sergeev added a comment to D59366: [NewPM][TimePasses] allow -time-passes reporting into a custom stream.

Thanks for yet another thorough review!

Mar 15 2019, 12:25 PM · Restricted Project
fedor.sergeev updated the diff for D59366: [NewPM][TimePasses] allow -time-passes reporting into a custom stream.

removing unneeded assert

Mar 15 2019, 9:31 AM · Restricted Project
fedor.sergeev added a parent revision for D59416: [Legacy][TimePasses] allow -time-passes reporting into a custom stream: D59366: [NewPM][TimePasses] allow -time-passes reporting into a custom stream.
Mar 15 2019, 8:03 AM · Restricted Project
fedor.sergeev added a child revision for D59366: [NewPM][TimePasses] allow -time-passes reporting into a custom stream: D59416: [Legacy][TimePasses] allow -time-passes reporting into a custom stream.
Mar 15 2019, 8:03 AM · Restricted Project
fedor.sergeev created D59416: [Legacy][TimePasses] allow -time-passes reporting into a custom stream.
Mar 15 2019, 8:02 AM · Restricted Project
fedor.sergeev updated the summary of D59366: [NewPM][TimePasses] allow -time-passes reporting into a custom stream.
Mar 15 2019, 6:58 AM · Restricted Project
fedor.sergeev updated the diff for D59366: [NewPM][TimePasses] allow -time-passes reporting into a custom stream.

test cleanup - removing unneeded stuff, adding a few more checks, adding comments

Mar 15 2019, 6:55 AM · Restricted Project
fedor.sergeev added a comment to D59366: [NewPM][TimePasses] allow -time-passes reporting into a custom stream.

Thanks for review!

Mar 15 2019, 6:16 AM · Restricted Project

Mar 14 2019

fedor.sergeev updated the summary of D59366: [NewPM][TimePasses] allow -time-passes reporting into a custom stream.
Mar 14 2019, 2:17 PM · Restricted Project
fedor.sergeev updated the diff for D59366: [NewPM][TimePasses] allow -time-passes reporting into a custom stream.

handling default case the same way as it was before;
adding unit-test for non-default case

Mar 14 2019, 2:14 PM · Restricted Project
fedor.sergeev planned changes to D59366: [NewPM][TimePasses] allow -time-passes reporting into a custom stream.

Okey, a proper solution here is to get back to not setting the stream by default and restort to old CreateInfoOutputFile-just-for-printing scheme.
This way we do not need to play with ownership of that info-output-file stream.

Mar 14 2019, 11:11 AM · Restricted Project
fedor.sergeev added a comment to D59366: [NewPM][TimePasses] allow -time-passes reporting into a custom stream.

CreateInfoOutputFile creates a new stream and unless we retake the ownership it will be destroyed.

True, but that's what it is doing right now. Do you see reason to change that?

Okey, let me enumerate all the possible solutions that I see:

  1. own stream in TimePassesHandler (my initial implementation)
  2. create TimePasses disabled/no-stream, enable from NewPMDriver (current state), own by NewPMDriver
  3. pass the stream to StandardInstrumentation constructor, pass through into TimePassesHandler constructor, own by NewPMDriver
  4. change behavior of CreateInfoOutputFile and make it handle the ownership itself
Mar 14 2019, 9:23 AM · Restricted Project
fedor.sergeev added a comment to D59366: [NewPM][TimePasses] allow -time-passes reporting into a custom stream.

Why not default to CreateInfoOutputFile inside of TimePassesHandler if OutputStream is unset? And then have a setStream(raw_ostream&) method to change it.

That is purely a chicken/egg issue.
CreateInfoOutputFile creates a new stream and unless we retake the ownership it will be destroyed.

Mar 14 2019, 9:09 AM · Restricted Project
fedor.sergeev updated the diff for D59366: [NewPM][TimePasses] allow -time-passes reporting into a custom stream.

outstream in TimePasses is set when enabling, so now it is not possible
to enable and have outstream == nullptr.

Mar 14 2019, 8:55 AM · Restricted Project
fedor.sergeev planned changes to D59366: [NewPM][TimePasses] allow -time-passes reporting into a custom stream.
Mar 14 2019, 8:38 AM · Restricted Project
fedor.sergeev updated the summary of D59366: [NewPM][TimePasses] allow -time-passes reporting into a custom stream.
Mar 14 2019, 8:25 AM · Restricted Project
fedor.sergeev updated the diff for D59366: [NewPM][TimePasses] allow -time-passes reporting into a custom stream.

As per discussion with @philip.pfaffe - owning output stream is rather strange for TimePasses object.
This ownership idea came from CreateInfoOutputFile design, as it returns new stream each time.

Mar 14 2019, 8:24 AM · Restricted Project
fedor.sergeev retitled D59366: [NewPM][TimePasses] allow -time-passes reporting into a custom stream from [TimePasses] allow -time-passes reporting into a custom stream to [NewPM][TimePasses] allow -time-passes reporting into a custom stream.
Mar 14 2019, 8:00 AM · Restricted Project
fedor.sergeev abandoned D58572: [Support] allow -stats/-time-passes reporting into a custom stream.

Abandoning this in favor of D59366.

Mar 14 2019, 7:56 AM · Restricted Project
fedor.sergeev created D59366: [NewPM][TimePasses] allow -time-passes reporting into a custom stream.
Mar 14 2019, 7:55 AM · Restricted Project

Feb 27 2019

fedor.sergeev requested changes to D56336: [Support] unflake TempFileCollisions test.

Agreed with @rsmith, running 32 times and collecting the amount of successes == 16 should be a fine way to do this.

Feb 27 2019, 3:56 AM · Restricted Project
fedor.sergeev accepted D58406: Fix IR/Analysis layering issue in OptBisect.

I do like the final result!
See my comment, but its good to go even w/o my suggestion.

Feb 27 2019, 3:50 AM · Restricted Project
fedor.sergeev added a comment to D58572: [Support] allow -stats/-time-passes reporting into a custom stream.

This change is focused purely on the stream that LLVM uses to report the data being collected.
No matter how many TimePassesHandlers you create (and yes, we do create TimePassesHandler one per thread-per-pipeline) they all emit into the same CreateInfoOutputFile() - either stderr or a single file.
I'm trying to solve a problem of a badly interleaved output, thats all.

You could set the output directly on the handler.

Do you suggest to get rid of the current scheme where we have a single output file for all kinds of "info data"?
Say, now w/o these changes -info-output-file is controlling the output file for both -stats and -time-passes.
Do you think it was a wrong design and we need to introduce separate output controls for each of those features?

Feb 27 2019, 1:45 AM · Restricted Project

Feb 25 2019

fedor.sergeev added a comment to D58572: [Support] allow -stats/-time-passes reporting into a custom stream.

I don't think this is sufficient or goes into the right direction to support parallel info collection and output.

For the new PassManager, reporting per-thread timings is trivial already and requires no (additional) global state. You can just create a TimePassesHandler instance per thread or per pipeline.

A global state of the data being collected is indeed a problem, yet I'm not trying to address it here.

Feb 25 2019, 4:03 PM · Restricted Project

Feb 22 2019

fedor.sergeev updated the diff for D58572: [Support] allow -stats/-time-passes reporting into a custom stream.

clang-formatted.
still figuring out how to unit-test this thing.

Feb 22 2019, 11:47 PM · Restricted Project
fedor.sergeev created D58572: [Support] allow -stats/-time-passes reporting into a custom stream.
Feb 22 2019, 11:45 PM · Restricted Project
fedor.sergeev added a comment to D58406: Fix IR/Analysis layering issue in OptBisect.

I don't feel too strongly from the available options - but hope someone with a closer connection/appreciation for these classes might have an opinion about whether adding the extra virtual function to Pass is problematic (philosophically or practically) or not.

And here will I chime yet again with connection to NewPM (where, say, Passes do not have virtual functions).

Feb 22 2019, 11:57 AM · Restricted Project

Feb 20 2019

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

Does the existing code successfully hoist the widenable_condition call itself out of the loop? If not, splitting out specifically that part of the transform would be well called for and necessary for unswitch to make progress. If it does, then the patch as written includes redundant handling which should be removed.

Current trunk can not hoist this widenable call.
Hence it makes sense to modify this fix so it hoists the call itself (and perhaps the rest of condition) but keeps the branch unmodified, for unswitch later to finish the task.

Feb 20 2019, 1:54 PM · Restricted Project
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 ;)

Feb 20 2019, 10:08 AM · Restricted Project
fedor.sergeev added a reviewer for D58406: Fix IR/Analysis layering issue in OptBisect: chandlerc.
Feb 20 2019, 8:20 AM · Restricted Project
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()))
Feb 20 2019, 8:19 AM · Restricted Project
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.

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

Feb 19 2019

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.

Feb 19 2019, 8:10 AM · Restricted Project

Feb 15 2019

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

Feb 15 2019, 3:41 AM · Restricted Project

Feb 12 2019

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!

Feb 12 2019, 12:30 AM · Restricted Project

Feb 11 2019

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.

Feb 11 2019, 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.

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

LGTM.

Feb 11 2019, 5:30 AM · Restricted Project

Feb 6 2019

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

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

Feb 6 2019, 6:41 AM · Restricted Project

Feb 4 2019

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

LGTM.

Feb 4 2019, 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.

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

LGTM bar the small nit below.

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

Jan 22 2019

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

Jan 22 2019, 12:59 AM

Jan 21 2019

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.

Jan 21 2019, 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.

Jan 21 2019, 2:53 PM