Page MenuHomePhabricator

philip.pfaffe (Philip Pfaffe)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 21 2015, 2:43 AM (204 w, 5 h)

Recent Activity

Fri, Mar 15

philip.pfaffe accepted D59366: [NewPM][TimePasses] allow -time-passes reporting into a custom stream.

Looks good!

Fri, Mar 15, 11:48 AM · Restricted Project
philip.pfaffe added a comment to D59366: [NewPM][TimePasses] allow -time-passes reporting into a custom stream.

Couple of nits inline, otherwise looks great!

Fri, Mar 15, 5:13 AM · Restricted Project

Thu, Mar 14

philip.pfaffe 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?

Thu, Mar 14, 9:13 AM · Restricted Project
philip.pfaffe 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.

Thu, Mar 14, 8:59 AM · Restricted Project

Tue, Mar 12

philip.pfaffe accepted D59258: [Docs] Add a note about legacy FunctionPassManager to the LLVM tutorial..

LGTM. Did you verify that there's no conflicting info in the first three chapters of the tutorial?

Tue, Mar 12, 8:33 AM · Restricted Project

Wed, Feb 27

philip.pfaffe added inline comments to D58406: Fix IR/Analysis layering issue in OptBisect.
Wed, Feb 27, 4:30 AM · Restricted Project
philip.pfaffe added a comment to D58572: [Support] allow -stats/-time-passes reporting into a custom stream.

Do you suggest to get rid of the current scheme where we have a single output file for all kinds of "info data"?

No I don't, that should certainly be the default.

Wed, Feb 27, 2:07 AM · Restricted Project
philip.pfaffe 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.

Wed, Feb 27, 1:23 AM · Restricted Project

Mon, Feb 25

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

Mon, Feb 25, 4:10 AM · Restricted Project

Wed, Feb 20

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

Wed, Feb 20, 9:08 AM · Restricted Project

Tue, Feb 19

philip.pfaffe abandoned D57793: [NewPM][MSan] Add sanitizer at O0.
Tue, Feb 19, 9:20 PM · Restricted Project

Feb 13 2019

philip.pfaffe accepted D56470: [NewPM] Second attempt at porting ASan.

Tiny final nit, otherwise looks great!

Feb 13 2019, 12:31 PM · Restricted Project, Restricted Project

Feb 11 2019

philip.pfaffe added a comment to D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting.

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.

Feb 11 2019, 3:16 AM · Restricted Project

Feb 9 2019

philip.pfaffe added a comment to D56470: [NewPM] Second attempt at porting ASan.

It's still missing the O0 part. I forgot adding it for msan and tsan, but we should do asan correctly from the start.

Feb 9 2019, 3:54 AM · Restricted Project, Restricted Project

Feb 5 2019

philip.pfaffe created D57793: [NewPM][MSan] Add sanitizer at O0.
Feb 5 2019, 3:18 PM · Restricted Project

Feb 4 2019

philip.pfaffe committed rG0ee6a933cec4: [NewPM][MSan] Add Options Handling (authored by philip.pfaffe).
[NewPM][MSan] Add Options Handling
Feb 4 2019, 1:03 PM
philip.pfaffe requested changes to D56470: [NewPM] Second attempt at porting ASan.

Sorry, there's something I missed, comment inline.

Feb 4 2019, 12:59 PM · Restricted Project, Restricted Project
philip.pfaffe updated the diff for D57640: [NewPM][MSan] Add Options Handling.

Add the default argument back.

Feb 4 2019, 12:45 PM · Restricted Project, Restricted Project

Feb 3 2019

philip.pfaffe committed rG9438585fe457: [DA][NewPM] Handle transitive dependencies in the new-pm version of DA (authored by philip.pfaffe).
[DA][NewPM] Handle transitive dependencies in the new-pm version of DA
Feb 3 2019, 4:28 AM

Feb 2 2019

philip.pfaffe accepted D56470: [NewPM] Second attempt at porting ASan.

Great! Just three more nits. Feel free to land after adressing them!

Feb 2 2019, 3:31 PM · Restricted Project, Restricted Project
philip.pfaffe added a comment to D56935: [NewPM] Add support for new-PM plugins to clang.

Landed it for you in r352972. Thanks!

Feb 2 2019, 3:20 PM · Restricted Project
philip.pfaffe created D57640: [NewPM][MSan] Add Options Handling.
Feb 2 2019, 12:49 PM · Restricted Project, Restricted Project

Jan 30 2019

philip.pfaffe added a comment to D56470: [NewPM] Second attempt at porting ASan.

This is starting to look great! Minor nits inline.

Jan 30 2019, 11:41 AM · Restricted Project, Restricted Project

Jan 25 2019

philip.pfaffe accepted D56935: [NewPM] Add support for new-PM plugins to clang.

It would be good to check, since the bots won't! Otherwise this looks good.

Jan 25 2019, 12:57 AM · Restricted Project
philip.pfaffe added inline comments to D56470: [NewPM] Second attempt at porting ASan.
Jan 25 2019, 12:49 AM · Restricted Project, Restricted Project

Jan 23 2019

philip.pfaffe added a comment to D56935: [NewPM] Add support for new-PM plugins to clang.

I'm not sure what the current state of plugins on windows is. They were broken and disabled last time I worked on this, but that might've changed in the meantime! Worth checking.

Jan 23 2019, 3:32 AM · Restricted Project
philip.pfaffe added a comment to D56470: [NewPM] Second attempt at porting ASan.

One thing I missed.

Jan 23 2019, 3:19 AM · Restricted Project, Restricted Project
philip.pfaffe added a comment to D56470: [NewPM] Second attempt at porting ASan.

Nice, we're getting somewhere! A lot of comments inline. Most importantly, your analysis is a function analysis, which it mustn't be, otherwise there's no gain from it at all.

Jan 23 2019, 3:15 AM · Restricted Project, Restricted Project

Jan 22 2019

philip.pfaffe added a comment to D56935: [NewPM] Add support for new-PM plugins to clang.

This generally looks sane. What will happen on windows though? Will it silently fail?

Jan 22 2019, 12:13 PM · Restricted Project

Jan 18 2019

philip.pfaffe added a comment to D56470: [NewPM] Second attempt at porting ASan.

I ran a benchmark using asan on a synthetic testcase made up of 10k functions and 10k globals, and it's clearly visible that GlobalsMD causes the performance problem. This is because during initialization it looks at all the globals registered in llvm.asan.globals, which is used to mark the globals coming from the frontend which are supposed to be instrumented. The function pass, however, doesn't use most of this, and accesses GlobalsMD only within AddressSanitizer::GlobalIsLinkerInitialized to check whether a global is dynamically initialized. This means we can fix this in two ways:

Jan 18 2019, 9:01 AM · Restricted Project, Restricted Project

Jan 17 2019

philip.pfaffe created D56846: [NewPM] Catch sanitizers passed to -fsanitize that haven't been implemented.
Jan 17 2019, 4:39 AM
philip.pfaffe created D56831: [NewPM] Add -fsanitize={memory,thread} handling to clang.
Jan 17 2019, 1:54 AM

Jan 16 2019

philip.pfaffe added a comment to D56433: [NewPM] Port tsan.

@delcypher Thanks for pointing it out! Just landed a revised version in rL351314.

Jan 16 2019, 3:20 AM
philip.pfaffe updated the diff for D56734: [MSan] Apply the ctor creation scheme of TSan.

Address comments and update two missing testcases.

Jan 16 2019, 1:18 AM

Jan 15 2019

philip.pfaffe added a parent revision for D56734: [MSan] Apply the ctor creation scheme of TSan: D56538: [NewPM][TSan] Reiterate the TSan port.
Jan 15 2019, 10:57 AM
philip.pfaffe added a child revision for D56538: [NewPM][TSan] Reiterate the TSan port: D56734: [MSan] Apply the ctor creation scheme of TSan.
Jan 15 2019, 10:57 AM
philip.pfaffe created D56734: [MSan] Apply the ctor creation scheme of TSan.
Jan 15 2019, 10:56 AM
philip.pfaffe updated the diff for D56538: [NewPM][TSan] Reiterate the TSan port.

Execute appendtoGlobalCtors lazily as well. Previously the inserted ctor was
still added multiple times to the global ctors list. Doing this lazily requires
knowing when the ctor is actually created, so I added a callback to handle the
insertion.

Jan 15 2019, 10:51 AM

Jan 11 2019

philip.pfaffe updated the diff for D56538: [NewPM][TSan] Reiterate the TSan port.

Insert the constructor and init functions only once per module.

Jan 11 2019, 3:44 AM
philip.pfaffe added inline comments to D56538: [NewPM][TSan] Reiterate the TSan port.
Jan 11 2019, 2:08 AM
philip.pfaffe updated the diff for D56538: [NewPM][TSan] Reiterate the TSan port.

Fix a comment.

Jan 11 2019, 2:04 AM

Jan 10 2019

philip.pfaffe updated the diff for D56538: [NewPM][TSan] Reiterate the TSan port.

Add missing license to TSan header.

Jan 10 2019, 4:20 AM
philip.pfaffe created D56538: [NewPM][TSan] Reiterate the TSan port.
Jan 10 2019, 4:16 AM
philip.pfaffe accepted D53895: [LoopUnroll] add parsing for unroll parameters in -passes pipeline.

Looks good to me, thanks!

Jan 10 2019, 12:44 AM

Jan 8 2019

philip.pfaffe accepted D56452: Fix go bindings for r350647: missed a function rename.

Thanks! Sorry that I missed this.

Jan 8 2019, 1:48 PM
philip.pfaffe updated the diff for D56381: [DA][NewPM] Handle transitive dependencies in the new-pm version of DA.

Rebase (and drop changes from D56386 that snuck in).

Jan 8 2019, 6:25 AM · Restricted Project
philip.pfaffe created D56433: [NewPM] Port tsan.
Jan 8 2019, 4:40 AM

Jan 7 2019

philip.pfaffe updated the diff for D56381: [DA][NewPM] Handle transitive dependencies in the new-pm version of DA.

Rebase onto D56386 and add testcase for DA invalidation

Jan 7 2019, 4:32 AM · Restricted Project
philip.pfaffe created D56386: [DA][NewPM] Add a printerpass and port the testsuite.
Jan 7 2019, 4:30 AM
philip.pfaffe created D56381: [DA][NewPM] Handle transitive dependencies in the new-pm version of DA.
Jan 7 2019, 3:17 AM · Restricted Project

Jan 5 2019

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

Cool! Down to bikeshed comments :)

Jan 5 2019, 8:19 AM

Jan 3 2019

philip.pfaffe added a comment to D54337: [ASan] Make AddressSanitizer a ModulePass.

I haven't gotten back to this yet, but will do so after looking into what may be required for also porting SafeStack and ShadowCallStack to new PM.

If I recall correctly, it was pointed out on the SafeStack thread that, since SafeStack is part of CodeGen, porting it is not possible right now.

Jan 3 2019, 10:50 AM · Restricted Project, Restricted Project
philip.pfaffe added inline comments to D53895: [LoopUnroll] add parsing for unroll parameters in -passes pipeline.
Jan 3 2019, 7:46 AM
philip.pfaffe added a comment to D54337: [ASan] Make AddressSanitizer a ModulePass.

@leonardchan Are you making any progress here?

Jan 3 2019, 6:16 AM · Restricted Project, Restricted Project
philip.pfaffe abandoned D54394: [WIP][NewPM] Port msan.

Superseded by D55647.

Jan 3 2019, 6:03 AM
philip.pfaffe added inline comments to D53895: [LoopUnroll] add parsing for unroll parameters in -passes pipeline.
Jan 3 2019, 3:09 AM

Jan 2 2019

philip.pfaffe updated the diff for D55647: [NewPM] Port Msan: Second alternative approach without any module level initalization.

Addressed comments and rebased on top of rL350219.

Jan 2 2019, 10:12 AM
philip.pfaffe updated the diff for D56130: Extend Module::getOrInsertGlobal to also take the default arguments accepted by the GlobalVariable constructor..

Follow @chandlerc's proposal for a much more narrow API.

Jan 2 2019, 3:27 AM

Dec 29 2018

philip.pfaffe added a comment to D56130: Extend Module::getOrInsertGlobal to also take the default arguments accepted by the GlobalVariable constructor..

That solves the default case, but I feel like the new three-argument case is still not much better than getGlobalVariable and an if.

Dec 29 2018, 3:32 PM
philip.pfaffe added a comment to D56130: Extend Module::getOrInsertGlobal to also take the default arguments accepted by the GlobalVariable constructor..

I get the idea of trying to separate query and creation arguments to make their meaning explicit. But how do you expect this to be used? The default call (i.e., the one everyone is using right now), would then look like

M.getOrInsertGlobal("Global", Ctx.getIntTy(), [] { return new GlobalVariable("Global", Ctx.getIntTy()); });

That's super verbose, and to me at least it's much harder to read then just calling Module::getGlobalVariable in an if.

Dec 29 2018, 2:47 PM

Dec 28 2018

philip.pfaffe added a comment to D56130: Extend Module::getOrInsertGlobal to also take the default arguments accepted by the GlobalVariable constructor..

The extra arguments are not filters. The idea here is to basicly be a mostly-drop-in replacement for calling the GlobalVariable constructor.

Dec 28 2018, 9:07 AM
philip.pfaffe created D56130: Extend Module::getOrInsertGlobal to also take the default arguments accepted by the GlobalVariable constructor..
Dec 28 2018, 5:34 AM

Dec 27 2018

philip.pfaffe added inline comments to D55647: [NewPM] Port Msan: Second alternative approach without any module level initalization.
Dec 27 2018, 5:53 AM

Dec 24 2018

philip.pfaffe added a comment to D55605: [NewPM] Port Msan: Alternative approach using a module to create the global constructor.

A couple of nits.
I also think the newly added classes should be marked as such, not as structs, but YMMV.

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

Absolutely :)

Dec 24 2018, 4:33 AM
philip.pfaffe updated the diff for D55647: [NewPM] Port Msan: Second alternative approach without any module level initalization.

Address comments:

  • Move init creator function next to the ctor function
  • Use Module::getOrInsertGlobal instead of my own version, but modify it to fit sanitizer neads
Dec 24 2018, 4:32 AM

Dec 23 2018

philip.pfaffe abandoned D55605: [NewPM] Port Msan: Alternative approach using a module to create the global constructor.

Abandoning in favor of D55605.

Dec 23 2018, 4:20 AM

Dec 20 2018

philip.pfaffe accepted D55278: [NewPM] -print-module-scope -print-after now prints module even after invalidated Loop/SCC.

Thanks, LGTM!

Dec 20 2018, 6:18 AM
philip.pfaffe added inline comments to D55278: [NewPM] -print-module-scope -print-after now prints module even after invalidated Loop/SCC.
Dec 20 2018, 4:37 AM
philip.pfaffe accepted D51748: cmake: Remove add_llvm_loadable_module().

This looks mostly like a mechanical change now, so LGTM!

Dec 20 2018, 2:28 AM

Dec 17 2018

philip.pfaffe added inline comments to D55278: [NewPM] -print-module-scope -print-after now prints module even after invalidated Loop/SCC.
Dec 17 2018, 1:40 PM
philip.pfaffe added a comment to D55278: [NewPM] -print-module-scope -print-after now prints module even after invalidated Loop/SCC.

This generally looks fine, some code comments inline.

Dec 17 2018, 1:30 PM

Dec 14 2018

philip.pfaffe added a comment to D54337: [ASan] Make AddressSanitizer a ModulePass.

It depends on what you mean by task. There is no clean way for depending on changes between IR units. I.e. you can't express the requirement that two passes have to run in a specific order. But that's okay, implementing such requirements would incure insane complexity. Fortunately, if I recall correctly Asan oesn't need that! Correct me if I'm wrong.

Dec 14 2018, 2:38 AM · Restricted Project, Restricted Project
philip.pfaffe added a comment to D55605: [NewPM] Port Msan: Alternative approach using a module to create the global constructor.

I dropped MSanOptions over in D55647 (or didn't need it there), which will likely supersede this change here.

Dec 14 2018, 2:12 AM
philip.pfaffe added inline comments to D55647: [NewPM] Port Msan: Second alternative approach without any module level initalization.
Dec 14 2018, 2:11 AM

Dec 13 2018

philip.pfaffe created D55647: [NewPM] Port Msan: Second alternative approach without any module level initalization.
Dec 13 2018, 3:18 AM

Dec 12 2018

philip.pfaffe updated the summary of D55605: [NewPM] Port Msan: Alternative approach using a module to create the global constructor.
Dec 12 2018, 10:04 AM
philip.pfaffe created D55605: [NewPM] Port Msan: Alternative approach using a module to create the global constructor.
Dec 12 2018, 9:33 AM

Dec 11 2018

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

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:44 AM

Dec 10 2018

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

Dec 10 2018, 1:59 PM
philip.pfaffe 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?

Dec 10 2018, 1:29 PM
philip.pfaffe accepted D54740: [NewPM] fixing asserts on deleted loop in -print-after-all.
Dec 10 2018, 7:05 AM
philip.pfaffe added a comment to D54740: [NewPM] fixing asserts on deleted loop in -print-after-all.

Thanks, this does LGTM!

Dec 10 2018, 5:36 AM

Dec 7 2018

philip.pfaffe added inline comments to D54740: [NewPM] fixing asserts on deleted loop in -print-after-all.
Dec 7 2018, 6:39 AM

Dec 6 2018

philip.pfaffe 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?

Dec 6 2018, 9:42 AM

Dec 5 2018

philip.pfaffe added a comment to D54394: [WIP][NewPM] Port msan.

I think I'm missing some context here.
What is the problem you are trying to solve?

msan as most sanitizers creates a new function to initialize itself, which function passes cannot do.

Technically they can, at least now. Are there any upcoming changes to that?

Dec 5 2018, 6:44 AM
philip.pfaffe 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?

Having a pass introduces dependencies between passes, which I find unsetteling. I.e. when you use the asan pass, you _must_ also use the asan-init pass, or otherwise you end up with IR that you can't compile without linker errors. That being said, the same is of course true when using the command line argument approach, so asan-init might not be the worst at the end of the day.

Dec 5 2018, 6:43 AM

Dec 3 2018

philip.pfaffe added inline comments to D54740: [NewPM] fixing asserts on deleted loop in -print-after-all.
Dec 3 2018, 4:43 AM

Nov 21 2018

philip.pfaffe 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?

Nov 21 2018, 2:04 PM

Nov 16 2018

philip.pfaffe added a comment to D54337: [ASan] Make AddressSanitizer a ModulePass.

Sorry for the delayed response. So the main problem is that AddressSantizer needs to perform some initialization involving reading stuff from global metadata and getting some target specific information. More specifically, just these 6 things which are all initialized in doInitialization:

GlobalsMD.init(M);
C = &(M.getContext());
LongSize = M.getDataLayout().getPointerSizeInBits();
IntptrTy = Type::getIntNTy(*C, LongSize);
TargetTriple = Triple(M.getTargetTriple());
Mapping = getShadowMapping(TargetTriple, LongSize, CompileKernel);

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.

Nov 16 2018, 1:17 AM · Restricted Project, Restricted Project

Nov 13 2018

philip.pfaffe retitled D54394: [WIP][NewPM] Port msan from [NewPM] Port msan to [WIP][NewPM] Port msan.
Nov 13 2018, 11:08 AM

Nov 11 2018

philip.pfaffe created D54394: [WIP][NewPM] Port msan.
Nov 11 2018, 10:28 AM

Nov 10 2018

philip.pfaffe added a comment to D54337: [ASan] Make AddressSanitizer a ModulePass.

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

I might have been wrong here, I assumed the Asan function pass did this,too, because all sanitizers I've looked at do it. But it's possible that only the Module variant does it.

Nov 10 2018, 4:02 AM · Restricted Project, Restricted Project
philip.pfaffe 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. So we need to initialize out-of-band, either with a separate module pass, or some function outside of the pass pipeline. I strongly prefer the latter, but it's not entirely clear to me how this function should be called. It's obviousl when running this through clang, but what about opt?

Nov 10 2018, 3:01 AM · Restricted Project, Restricted Project
philip.pfaffe 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.

Nov 10 2018, 2:25 AM
philip.pfaffe 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?

Nov 10 2018, 1:18 AM · Restricted Project, Restricted Project
philip.pfaffe created D54374: [newpm] Add an OptimizerLast EP.
Nov 10 2018, 1:08 AM

Nov 6 2018

philip.pfaffe added a comment to D53376: Fix generation of exported targets in build directory.

Commited!

Nov 6 2018, 7:21 AM

Nov 2 2018

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

What is the advantage of this over using command line args?

Nov 2 2018, 9:54 AM

Oct 30 2018

philip.pfaffe abandoned D49486: [cfe][CMake] Export the clang resource directory.

It looks like running clang will be good enough. I'm closing this for now!

Oct 30 2018, 4:50 AM