philip.pfaffe (Philip Pfaffe)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 21 2015, 2:43 AM (178 w, 3 d)

Recent Activity

Today

philip.pfaffe added a comment to D50923: [New PM][PassInstrumentation] IR printing support for New Pass Manager.

See my comment to D52329. Analysis isn't the right place for this implementation. This should be part of the Passes library, and even live directly in the StandardInstrumentations implementation.

Fri, Sep 21, 1:27 AM
philip.pfaffe added a comment to D51276: [New PM][PassTiming] implement -time-passes for the new pass manager.

Some inline comments.

Fri, Sep 21, 1:23 AM
philip.pfaffe added a comment to D52329: [New PM] adding helper that manages standard instrumentations.

This looks nice! At this point, I think it makes sense to just move the entire IR Printing implementation here. I don't even think it needs to be mentioned in the header. It doesn't have any other clients, and Analysis is definitely the wrong home.

Fri, Sep 21, 1:02 AM

Wed, Sep 19

philip.pfaffe added a comment to D51632: [New PM][PassInstrumentation] enhancing PassInstrumentation with PassManager tracking.

I don't fully understand the rationale behind this. How is this different compared to a before/after instrumentation matching the PassManagers?

Wed, Sep 19, 2:55 AM
philip.pfaffe accepted D47858: [New PM] Introducing PassInstrumentation framework.

So I'd still prefer a generic free getAnalysisResult, plus specializations for the non-simple cases, e.g. Loop and CGSCC. That way there's less surprises should there ever be a 'weird' AnalysisManager out there using a different argument mapping. Happy to do this in a followup though!

Wed, Sep 19, 12:42 AM

Tue, Sep 18

philip.pfaffe added inline comments to D47858: [New PM] Introducing PassInstrumentation framework.
Tue, Sep 18, 2:37 PM

Sat, Sep 15

philip.pfaffe added inline comments to D47858: [New PM] Introducing PassInstrumentation framework.
Sat, Sep 15, 8:46 AM

Fri, Sep 14

philip.pfaffe added a comment to D50923: [New PM][PassInstrumentation] IR printing support for New Pass Manager.

I want all the debugging options to work the same manner whether it is opt or any other tool/way to run the pipeline
(say, in our JIT compiler we do sometimes use -print options). And I want them to work the same way both in legacy and new pass manager.
That means the main controlling interface should better be joined (and thats where ShouldPrint come into play).

Note that I'm not saying we should abandon the -print command line options, just that the ShouldPrint functions are the entirely wrong place to access them. This indirection is simply not necessary anymore.

Fri, Sep 14, 3:25 PM
philip.pfaffe added a comment to D50923: [New PM][PassInstrumentation] IR printing support for New Pass Manager.

This implementation is modeled unnecessarily close to the legacy behavior. opt should just manually add the printer instrumentation if a print option is given. In particular., this shouldn't use the ShouldPrint* functions. This way there's no need to involve the PassBuilder API, and you also don't pay for the instrumentation if you don't want to print.

Fri, Sep 14, 2:32 PM
philip.pfaffe accepted D51275: [New PM][PassInstrumentation] Adding PassInstrumentation to the AnalysisManager runs.

This looks entirely straightforward.

Fri, Sep 14, 1:54 PM
philip.pfaffe added a comment to D47858: [New PM] Introducing PassInstrumentation framework.

At this point I'm bikeshedding: Should getAnalysisResultgo into the Internal header?

Fri, Sep 14, 8:53 AM
philip.pfaffe added a comment to D47858: [New PM] Introducing PassInstrumentation framework.

This is looking great. Apart from two things that aren't needed anymore, the last point of discussion is getPassInstrumentation. PassInstrumentationAnalysis shouldn't be a special citizen. What about making this a free function instead? In fact, why not make this a generic helper e.g. in the detail namespace to query _any_ analysis result from a template pass?

Fri, Sep 14, 6:03 AM

Thu, Sep 13

philip.pfaffe added a comment to D51973: [New PM] adding unit tests for PassInstrumentation.

This looks good! I dislike the AnyNumber() expecations you highlighted above, but I see that it's unavoidable without either repeating a lot of the test logic again, or without waiting for something like D39678 to happen.

Thu, Sep 13, 4:37 AM
philip.pfaffe added inline comments to D47858: [New PM] Introducing PassInstrumentation framework.
Thu, Sep 13, 4:33 AM

Thu, Sep 6

philip.pfaffe added inline comments to D51748: cmake: Refactor add_llvm_loadable_module().
Thu, Sep 6, 3:03 PM
philip.pfaffe added a comment to D47858: [New PM] Introducing PassInstrumentation framework.

I like how simple this is now!

Thu, Sep 6, 10:30 AM

Wed, Sep 5

philip.pfaffe added a comment to D47858: [New PM] Introducing PassInstrumentation framework.

Here's what I think the ownership layering should look like:

Wed, Sep 5, 3:07 AM

Mon, Sep 3

philip.pfaffe added inline comments to D47858: [New PM] Introducing PassInstrumentation framework.
Mon, Sep 3, 2:25 PM
philip.pfaffe added inline comments to D47858: [New PM] Introducing PassInstrumentation framework.
Mon, Sep 3, 1:41 PM
philip.pfaffe added inline comments to D47858: [New PM] Introducing PassInstrumentation framework.
Mon, Sep 3, 12:45 PM
philip.pfaffe added a comment to D47858: [New PM] Introducing PassInstrumentation framework.

I meant both your arguments towards passing the real IRUnit and an opaque pass identifier.

Mon, Sep 3, 12:37 PM
philip.pfaffe added a comment to D47858: [New PM] Introducing PassInstrumentation framework.

Okay, that sounds reasonable.

Mon, Sep 3, 8:39 AM
philip.pfaffe added a comment to D47858: [New PM] Introducing PassInstrumentation framework.

Reconsidering passing the actual IRUnit to the callbacks, I still think that's not a good idea. Downstream users _will_ eventually use this to hook into the pipeline and mess with the IR somehow. So instead this should be opaque to clients.

Mon, Sep 3, 4:06 AM
philip.pfaffe added a comment to D47858: [New PM] Introducing PassInstrumentation framework.

IMO passing void* as an opaque ID along with the name is fine. Passing the PassConcept by reference would degrade layering, since it lives in IR, not in Passes like the rest of this.

Mon, Sep 3, 3:41 AM
philip.pfaffe added a comment to D51276: [New PM][PassTiming] implement -time-passes for the new pass manager.

Instead of using a global timer registry, this should get a per-compilation registry from the PassExecutionCounter.

Mon, Sep 3, 3:22 AM

Wed, Aug 29

philip.pfaffe added a comment to D47858: [New PM] Introducing PassInstrumentation framework.

Back from vacation, and had time for a closer look.

Wed, Aug 29, 8:30 AM

Aug 21 2018

philip.pfaffe added a comment to D47858: [New PM] Introducing PassInstrumentation framework.

Thanks for moving this forward!

Aug 21 2018, 4:26 AM

Aug 16 2018

philip.pfaffe added a comment to D50668: unittests: Don't install TestPlugin.so.

We probably should just disable the unittest on windows, in particular if(NOT LLVM_ENABLE_PLUGINS AND NOT (ARG_PLUGIN_TOOL AND LLVM_EXPORT_SYMBOLS_FOR_PLUGINS)).

Aug 16 2018, 11:07 AM

Aug 15 2018

philip.pfaffe accepted D50668: unittests: Don't install TestPlugin.so.

Cool, LGTM!

Aug 15 2018, 2:42 AM

Aug 14 2018

philip.pfaffe added a comment to D50668: unittests: Don't install TestPlugin.so.

This makes complete sense to me, but I'm wondering whether add_llvm_loadable_module is the right macro to use here in the first place. All it does is call add_llvm_library with a MODULE argument, and set up the install.

Aug 14 2018, 2:17 AM

Aug 9 2018

philip.pfaffe added a comment to D49486: [cfe][CMake] Export the clang resource directory.

I had not, but it should work just as well!

Aug 9 2018, 10:03 AM
philip.pfaffe added a comment to D49486: [cfe][CMake] Export the clang resource directory.

Ping.

Aug 9 2018, 8:18 AM

Aug 2 2018

philip.pfaffe added a comment to D49024: [Polly] [WIP] Introduce ShapeInfo into polly for sizes and strides..

On top of my generic concern regarding the unclean implementation of the ShapeInfo itself, I left you a bunch of inline comments, mostly concerning style.

Aug 2 2018, 6:48 AM

Jul 27 2018

philip.pfaffe added a comment to D49872: [Polly][ScopDetect] Add option -polly-allow-nonaffine-read and enable by default..

Does this still allow disproving data dependences?

Jul 27 2018, 2:00 AM · Restricted Project

Jul 26 2018

philip.pfaffe added a comment to D49843: [CMake] Followup for r337366: Only export LLVM_LINK_LLVM_DYLIB it it's set to ON.

Since the 8.0 branch is around the corner I'd really like feedback from @beanz whether this makes sense or not.

Jul 26 2018, 4:06 AM
philip.pfaffe created D49843: [CMake] Followup for r337366: Only export LLVM_LINK_LLVM_DYLIB it it's set to ON.
Jul 26 2018, 4:05 AM

Jul 25 2018

philip.pfaffe added a comment to D49609: [isl] Typesafe user pointers.

The link is 404, but I assume it boils down to the same thing as either std::any or llvm::Any. It's not hard to reroll the implementation in isl, but then we have three.

Jul 25 2018, 2:58 AM
philip.pfaffe added a comment to D49609: [isl] Typesafe user pointers.

As we discussed offline, std::any doesn't really help polly, because it's a c++17 feature. Instead, what about making the generator itself extensible? Allow users to hook into the generation process and modify whatever happens. Then we can just emit llvm::Any privately. Additionally, this can settle the exceptions/no-exceptions debate.

Jul 25 2018, 1:34 AM

Jul 23 2018

philip.pfaffe updated the diff for D49609: [isl] Typesafe user pointers.

Fixery to appease the regression tests:

  • Back out of cool new api in PPCGCodeGeneration.
  • Don't delete Any through void*
  • Fix a get_user<const>
Jul 23 2018, 6:15 AM
philip.pfaffe updated the diff for D49609: [isl] Typesafe user pointers.

Address @bollu's comments

Jul 23 2018, 4:31 AM
philip.pfaffe updated the diff for D49609: [isl] Typesafe user pointers.

Handle nullptr users correctly

Jul 23 2018, 4:27 AM

Jul 20 2018

philip.pfaffe updated the diff for D49609: [isl] Typesafe user pointers.

Fail harder if the wrong type is given.

Jul 20 2018, 12:10 PM
philip.pfaffe updated the diff for D49609: [isl] Typesafe user pointers.

Missed a couple of c-api uses

Jul 20 2018, 11:15 AM
philip.pfaffe created D49609: [isl] Typesafe user pointers.
Jul 20 2018, 10:44 AM
philip.pfaffe added a comment to D49021: [Polly][isl] Simplify iterator implementation by building on top of list accessors.

Ping.

Jul 20 2018, 4:21 AM
philip.pfaffe added a comment to D29488: [DA] Fix for PR31848: Treat AddRec subscripts containing extra loops as NonLinear.

Realized this is still pending. Added some reviewers.

Jul 20 2018, 3:49 AM
philip.pfaffe added reviewers for D29488: [DA] Fix for PR31848: Treat AddRec subscripts containing extra loops as NonLinear: dmgreen, fhahn, hfinkel.
Jul 20 2018, 3:49 AM

Jul 18 2018

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

Abandoning in favor D49486, which subscribed the right list :)

Jul 18 2018, 7:45 AM
philip.pfaffe created D49486: [cfe][CMake] Export the clang resource directory.
Jul 18 2018, 7:44 AM
philip.pfaffe created D49478: [cfe][CMake] Export the clang resource directory.
Jul 18 2018, 5:35 AM

Jul 17 2018

philip.pfaffe accepted D49022: [Polly-ACC] disallow managed memory code generation for OpenCL.

Thanks! LGTM.

Jul 17 2018, 6:40 AM
philip.pfaffe added a comment to D49022: [Polly-ACC] disallow managed memory code generation for OpenCL.

Thank, only nits left!

Jul 17 2018, 5:51 AM

Jul 13 2018

philip.pfaffe added a comment to D49019: [Polly][isl] Add neutrally-named accessors to isl list elements and sizes.

Should I commit this change, or wait until the generator is updated and recreate it using that?

Jul 13 2018, 6:05 AM

Jul 11 2018

philip.pfaffe created D49193: [CMake] Export the LLVM_LINK_LLVM_DYLIB setting.
Jul 11 2018, 8:56 AM

Jul 10 2018

philip.pfaffe added a comment to D49019: [Polly][isl] Add neutrally-named accessors to isl list elements and sizes.

Is the generator in the repo?

Jul 10 2018, 7:32 AM
philip.pfaffe added a comment to D49022: [Polly-ACC] disallow managed memory code generation for OpenCL.

I'd prefer putting it into PPCGCodeGeneration.cpp/PPCGCodeGeneration.h.

Jul 10 2018, 7:10 AM
philip.pfaffe added a comment to D49130: Fix OpenCL Work-Item function arguments in Polly.

What's the relationship between this and D48774?

Jul 10 2018, 7:08 AM · Restricted Project

Jul 6 2018

philip.pfaffe added a comment to D49019: [Polly][isl] Add neutrally-named accessors to isl list elements and sizes.

Where's the code for the generator?

Jul 6 2018, 9:27 AM
philip.pfaffe added a comment to D49024: [Polly] [WIP] Introduce ShapeInfo into polly for sizes and strides..

I raised this concern already on Siddharth's version, but if this will succeed that I'll need to repeat it hear: ShapeInfo is pretty spaghetti, this should be fundamentally redesigned.

Jul 6 2018, 9:20 AM
philip.pfaffe added a comment to D49022: [Polly-ACC] disallow managed memory code generation for OpenCL.

I don't like that this messes with global state. This destroys local reasoning and makes refactoring unnecessarily harder in the future.

Jul 6 2018, 9:15 AM
philip.pfaffe created D49021: [Polly][isl] Simplify iterator implementation by building on top of list accessors.
Jul 6 2018, 5:41 AM
philip.pfaffe added a dependent revision for D49019: [Polly][isl] Add neutrally-named accessors to isl list elements and sizes: D49021: [Polly][isl] Simplify iterator implementation by building on top of list accessors.
Jul 6 2018, 5:41 AM
philip.pfaffe created D49019: [Polly][isl] Add neutrally-named accessors to isl list elements and sizes.
Jul 6 2018, 4:54 AM
philip.pfaffe added a comment to D46578: Fix gdb pretty printers to work with Python 3..

Ping

Jul 6 2018, 3:39 AM

Jul 3 2018

philip.pfaffe added a comment to D48883: [Polly-ACC] Add isl_space.h to gpu_tree.c.

Thanks for fixing this!

Jul 3 2018, 8:45 AM
philip.pfaffe added a comment to D48874: [Polly] [WIP] Introduce ShapeInfo into polly..

Before going into the details and nits:

Jul 3 2018, 5:02 AM · Restricted Project

Jun 29 2018

philip.pfaffe accepted D48774: [polly-acc] change cl_get_* return types to 32/64bit.

LGTM, thanks!

Jun 29 2018, 11:46 AM
philip.pfaffe added a comment to D48651: [RFC] Pattern matching on schedule trees..

I don't know when and how mainline isl will provide maintained C++ bindings, but the idea there is that enum isl_schedule_node_type will not be visible from C++. This is essentially a trick to have "inheritance" on node types in C. Practically, isl is going towards individual node types being specific classes that inherit from the generic schedule_node one. They are likely to feature isa<>-style interface. We can declare a local enum and use it to construct matchers though.

Is there a roadmap or RFC for that? I think that's a great idea.

Jun 29 2018, 3:32 AM · Restricted Project

Jun 27 2018

philip.pfaffe added a comment to D48651: [RFC] Pattern matching on schedule trees..

This was how my API worked:

Jun 27 2018, 10:13 AM · Restricted Project
philip.pfaffe added a comment to D48651: [RFC] Pattern matching on schedule trees..

I like the idea! The API needs to be polished a bit I think.

Jun 27 2018, 10:02 AM · Restricted Project

Jun 26 2018

philip.pfaffe added a comment to D48026: [ScopHelper] Provide support for recognising collective invariant loads.

OK, this looks good! You haven't addressed Micheal's testcase yet for the invariant cascade crossing alias set boundaries. For the sake of progress IMO it's okay to leave a TODO and address it in a followup change. @Meinersbur what do you think?

Jun 26 2018, 7:30 AM
philip.pfaffe added inline comments to D48026: [ScopHelper] Provide support for recognising collective invariant loads.
Jun 26 2018, 4:18 AM
philip.pfaffe added inline comments to D48026: [ScopHelper] Provide support for recognising collective invariant loads.
Jun 26 2018, 3:05 AM

Jun 25 2018

philip.pfaffe accepted D48026: [ScopHelper] Provide support for recognising collective invariant loads.

Thanks, LGTM!

Jun 25 2018, 4:49 AM

Jun 24 2018

philip.pfaffe added a comment to D48026: [ScopHelper] Provide support for recognising collective invariant loads.

There is still some clutter left, such as irrelevant metadata, a function declaration, and some computations, such as the value stored to memory.

Jun 24 2018, 4:08 AM

Jun 22 2018

philip.pfaffe added inline comments to D48136: [Polly] Implement an iterator for isl maps, basic_maps, sets, basic_sets.
Jun 22 2018, 5:09 AM
philip.pfaffe added a comment to D48026: [ScopHelper] Provide support for recognising collective invariant loads.

This is starting to look good! Only cosmetic nits left.

Jun 22 2018, 4:21 AM
philip.pfaffe added inline comments to D48026: [ScopHelper] Provide support for recognising collective invariant loads.
Jun 22 2018, 3:15 AM
philip.pfaffe added inline comments to D48026: [ScopHelper] Provide support for recognising collective invariant loads.
Jun 22 2018, 2:52 AM

Jun 21 2018

philip.pfaffe added inline comments to D48026: [ScopHelper] Provide support for recognising collective invariant loads.
Jun 21 2018, 6:43 AM

Jun 20 2018

philip.pfaffe added inline comments to D48136: [Polly] Implement an iterator for isl maps, basic_maps, sets, basic_sets.
Jun 20 2018, 4:01 AM
philip.pfaffe added inline comments to D48136: [Polly] Implement an iterator for isl maps, basic_maps, sets, basic_sets.
Jun 20 2018, 1:00 AM

Jun 19 2018

philip.pfaffe updated the diff for D48136: [Polly] Implement an iterator for isl maps, basic_maps, sets, basic_sets.

RAII management of the isl_ctx

Jun 19 2018, 3:54 AM
philip.pfaffe added inline comments to D48136: [Polly] Implement an iterator for isl maps, basic_maps, sets, basic_sets.
Jun 19 2018, 2:15 AM

Jun 18 2018

philip.pfaffe added inline comments to D48136: [Polly] Implement an iterator for isl maps, basic_maps, sets, basic_sets.
Jun 18 2018, 3:24 AM
philip.pfaffe updated the diff for D48136: [Polly] Implement an iterator for isl maps, basic_maps, sets, basic_sets.
Free the context in the unittest.
Jun 18 2018, 3:21 AM

Jun 14 2018

philip.pfaffe added reviewers for D48136: [Polly] Implement an iterator for isl maps, basic_maps, sets, basic_sets: Meinersbur, grosser.
Jun 14 2018, 2:04 AM
philip.pfaffe added a comment to D48136: [Polly] Implement an iterator for isl maps, basic_maps, sets, basic_sets.

Looks like I forgot to add pollydev as a subscriber. Shall I resubmit the patch?

Jun 14 2018, 2:03 AM
philip.pfaffe updated the diff for D48136: [Polly] Implement an iterator for isl maps, basic_maps, sets, basic_sets.
Add a unittest and address comments
Jun 14 2018, 2:01 AM

Jun 13 2018

philip.pfaffe added inline comments to D48136: [Polly] Implement an iterator for isl maps, basic_maps, sets, basic_sets.
Jun 13 2018, 2:36 PM
philip.pfaffe created D48136: [Polly] Implement an iterator for isl maps, basic_maps, sets, basic_sets.
Jun 13 2018, 10:36 AM

Jun 12 2018

philip.pfaffe created D48070: [Polly] Simplify the implementation of getCUDALibDeviceFunction. NFC..
Jun 12 2018, 3:19 AM

Jun 11 2018

philip.pfaffe added a comment to D48026: [ScopHelper] Provide support for recognising collective invariant loads.

As I mentioned in person: Instead of detecting this specific pattern, would it be possible to treat this as a fixpoint iteration? I.e., maintain a set of undecided address sources and keep shrinking it until only the non-invariant ones remein.

Jun 11 2018, 8:40 AM
philip.pfaffe added inline comments to D47930: Make email options of find_interesting_reviews more flexible..
Jun 11 2018, 2:45 AM

Jun 8 2018

philip.pfaffe updated the diff for D47890: [Polly][zorg] Enable GPGPU Codegen on the builders only if NVPTX is an LLVM target.

Fix weird iterator comprehension.

Jun 8 2018, 11:34 AM
philip.pfaffe added a comment to D47890: [Polly][zorg] Enable GPGPU Codegen on the builders only if NVPTX is an LLVM target.

I still think that any(True for ... is unnecessary.

Jun 8 2018, 1:11 AM

Jun 7 2018

philip.pfaffe updated the diff for D47890: [Polly][zorg] Enable GPGPU Codegen on the builders only if NVPTX is an LLVM target.

Add a comment

Jun 7 2018, 2:18 PM
philip.pfaffe added inline comments to D47890: [Polly][zorg] Enable GPGPU Codegen on the builders only if NVPTX is an LLVM target.
Jun 7 2018, 2:11 PM
philip.pfaffe created D47890: [Polly][zorg] Enable GPGPU Codegen on the builders only if NVPTX is an LLVM target.
Jun 7 2018, 9:24 AM
philip.pfaffe created D47888: [Polly] Back out of GPU Codegen if NVPTX is not available.
Jun 7 2018, 8:39 AM