Page MenuHomePhabricator

jdoerfert (Johannes Doerfert)
Argonne National Laboratory

Projects

User does not belong to any projects.

User Details

User Since
Jan 30 2014, 6:27 AM (311 w, 1 d)

Recent Activity

Today

jdoerfert added a comment to D72885: [WIP] Add Query API for llvm.assume holding attributes.

I won't review this properly till next week. Generally looks good so you can work on the next patch if you want to.

Fri, Jan 17, 4:17 PM · Restricted Project
jdoerfert added inline comments to D72475: [WIP] Build assume from call.
Fri, Jan 17, 4:17 PM · Restricted Project
jdoerfert accepted D72956: [libomptarget] Implement smid for amdgcn.

LGTM, I mean it is black magic but that is to be expected. One comment below.

Fri, Jan 17, 4:17 PM · Restricted Project
jdoerfert added inline comments to D72962: [MLIR, OpenMP] Translation of OpenMP barrier construct to LLVM IR.
Fri, Jan 17, 4:08 PM · Restricted Project
jdoerfert added inline comments to D72901: [OpenMP] [DOCS] Update OMP5.0 feature status table [NFC].
Fri, Jan 17, 11:02 AM

Yesterday

jdoerfert added inline comments to D72885: [WIP] Add Query API for llvm.assume holding attributes.
Thu, Jan 16, 7:13 PM · Restricted Project
jdoerfert added inline comments to D72475: [WIP] Build assume from call.
Thu, Jan 16, 5:47 PM · Restricted Project
jdoerfert added a comment to D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder..

The commit title has a stray { instead of [

Thu, Jan 16, 4:22 PM · Restricted Project, Restricted Project
jdoerfert added a comment to D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder..

Phab is not involved in the merge process, it just is a normal git push on top of the llmv-project.
Do you have commit rights already? If not I can commit this for you in a few days.

Thu, Jan 16, 4:12 PM · Restricted Project, Restricted Project
jdoerfert accepted D72884: [NFC] Factor out function to detect if an attribute has an argument..

LGTM with one small nit

Thu, Jan 16, 4:02 PM · Restricted Project
jdoerfert added a comment to D72382: [ArgPromotion] Extend search for SafeToUnconditionallyLoad indices to the blocks that must be executed upon entry into the function..

This should fix PR887: https://bugs.llvm.org/show_bug.cgi?id=887
Please verify that and include the test case from there. We should mention it in the commit message and close the bug if it works.

Thu, Jan 16, 12:47 PM · Restricted Project
jdoerfert added inline comments to D72798: [llvm][docs] LangRef for IR attribute `vector-function-abi-variants`..
Thu, Jan 16, 10:13 AM · Restricted Project
jdoerfert accepted D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder..

LGTM with some minor things you should address before the merge.

Thu, Jan 16, 1:03 AM · Restricted Project, Restricted Project

Wed, Jan 15

jdoerfert added a comment to D72798: [llvm][docs] LangRef for IR attribute `vector-function-abi-variants`..

What are instruction attributes? How would that look like? Is there a list discussion?

Wed, Jan 15, 12:32 PM · Restricted Project
jdoerfert added inline comments to D72542: [Flang][mlir] add a band-aid to support the creation of mutually recursive types when lowering to LLVM IR.
Wed, Jan 15, 12:32 PM · Restricted Project, Restricted Project
jdoerfert accepted D72789: [IR] Mark memset.* intrinsics as IntrWriteMem..

LGTM :)

Wed, Jan 15, 12:05 PM · Restricted Project
jdoerfert accepted D72789: [IR] Mark memset.* intrinsics as IntrWriteMem..

Can you comment on the test failures?

Wed, Jan 15, 11:09 AM · Restricted Project
jdoerfert added a comment to D71989: [OpenMP][IRBuilder][WIP] Prototype `omp task` support.

Having a copy function per type allows us to reuse it, otherwise we have one copy function per static task location (at worst). Either works for me I think.

I also would be OK with either option.
But note that using per-type functions will require more additions to the interface, like:

(..., num_objects, array_of_copy_wrappers, array_of_desctuctor_wrappers, array_of_obj_offsets).

Then the library can iterate over objects to copy-construct them, then iterate to destroy them after the task is complete. Without any possibility of inlining of any wrappers.

Per-task function only needs two additions - copy_wrapper and destructor_wrapper, all other details can live inside them, including possible inlining of constructors and destructors.

Wed, Jan 15, 11:09 AM · Restricted Project, Restricted Project
jdoerfert added a comment to D72789: [IR] Mark memset.* intrinsics as IntrWriteMem..

Can you comment on the test failures?

Wed, Jan 15, 11:09 AM · Restricted Project
jdoerfert added a comment to D70412: [OpenMP][Tool] Runtime warning for missing TSan-option.

This revision was not accepted before being committed!

It was a misunderstanding on my side. Johannes told me that he is fine with the changes before I pushed. I didn't check that the accepted flag was not on the patch.
What procedure do you suggest?

Wed, Jan 15, 2:28 AM · Restricted Project

Tue, Jan 14

jdoerfert added a comment to D71989: [OpenMP][IRBuilder][WIP] Prototype `omp task` support.

and we then call the copy constructors like this:

void *local_addr = ...;
for (kmp_uint32 u = 0; u < sizeof_copy_infos; ++u)
  local_addr = copy_wrapper_list[u](copied_obj_list[u], local_addr);

Just to confirm: that local_addr would be somehow linked to the task, I imagine it'd be initialized to something like task->shareds + offset_to_firstprivates, wouldn't it?

Tue, Jan 14, 6:00 PM · Restricted Project, Restricted Project
jdoerfert resigned from D72742: Don't assume promotable integers are zero/sign-extended already in x86-64 ABI..
Tue, Jan 14, 5:41 PM · Restricted Project, Restricted Project
jdoerfert added a comment to D72475: [WIP] Build assume from call.

i renamed the files to KnowledgeRetention.* to be more general. we can put other related utilities like the query API in the same files. i had a hard time finding a good name and i am not sure i succeeded what do you think ?

Tue, Jan 14, 5:32 PM · Restricted Project
jdoerfert added a comment to D72666: [IR] ArgMemOnly functions with WriteOnly ptr args do not read memory..

I'd prefer to just add writeonly markings in places where they're missing. For llvm.memset in particular, can we mark it IntrWriteMem?

Yes we should probably just mark llvm.memset as IntrWriteMem.

Tue, Jan 14, 4:12 PM · Restricted Project
jdoerfert accepted D71620: [Attributor] AAValueConstantRange: Value range analysis using constant range.

LGTM.

Tue, Jan 14, 12:50 PM · Restricted Project
jdoerfert added inline comments to D72542: [Flang][mlir] add a band-aid to support the creation of mutually recursive types when lowering to LLVM IR.
Tue, Jan 14, 11:32 AM · Restricted Project, Restricted Project
jdoerfert added a comment to D72525: [LIBOMPTARGET] Do not increment/decrement the refcount for "declare target" objects.

I'm fine with this. If others are, please accept and don't wait for me.

Tue, Jan 14, 11:32 AM · Restricted Project
jdoerfert accepted D72712: [nfc][libomptarget] Refactor amdgcn target_impl.

LGTM.

Tue, Jan 14, 11:23 AM · Restricted Project
jdoerfert accepted D72719: [nfc][libomptarget] Refactor nvptx/target_impl.cu.

LGTM.

Tue, Jan 14, 11:11 AM · Restricted Project
jdoerfert added inline comments to D71987: [OpenMP][NFC] Add a couple of TODOs to the runtime.
Tue, Jan 14, 11:02 AM · Restricted Project
jdoerfert added a comment to D71989: [OpenMP][IRBuilder][WIP] Prototype `omp task` support.

the task create and task issue step are
conceptually not separated anymore as it is

I don't think this can work reliably. Because not all C++ objects can be mem-copied.
E.g. an object can keep its own address or reference, and mem-copy will make it broken.
This could be fixed by generating (optional) thunk routine which would create all needed objects
in the library-allocated space, and similar routine which would destroy all the created objects.

Tue, Jan 14, 10:42 AM · Restricted Project, Restricted Project
jdoerfert added a comment to D69930: [OpenMP] Introduce the OpenMPOpt transformation pass.

Also, this is missing some statistics, and maybe some debug counters.

Tue, Jan 14, 10:42 AM · Restricted Project

Mon, Jan 13

jdoerfert accepted D72475: [WIP] Build assume from call.
`bool getAttributeFromAssume(Value &Base, CallInst &AssumeCI, Attribute::Kind Kind, int &Value)`

and similar for string attributes. Alternatively we could return a map from WhatOn values to attributes.

something like that seem fine.

I don't understand. What do you mean by "good names"? We have the operand bundle strings, right?

I meant that in the operand bundle the first argument is the WasOn and the second is optionally the attributes value.
but users shouldn't have to use magic numbers to access those fields.

Mon, Jan 13, 8:40 PM · Restricted Project
jdoerfert added a comment to D72666: [IR] ArgMemOnly functions with WriteOnly ptr args do not read memory..

I'd prefer to just add writeonly markings in places where they're missing. For llvm.memset in particular, can we mark it IntrWriteMem?

Mon, Jan 13, 6:04 PM · Restricted Project
jdoerfert added a comment to D72475: [WIP] Build assume from call.
Mon, Jan 13, 9:28 AM · Restricted Project

Sat, Jan 11

jdoerfert added a comment to D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder..

Commits need a message that explains the change.
Also the "Summary" should not be in the commit title.

Is this something for future commits or do you want me to change these?
If the latter, how do I do that?

Sat, Jan 11, 11:01 PM · Restricted Project, Restricted Project
jdoerfert retitled D71830: [OpenMP][Part 2] Use reusable OpenMP context/traits handling from [OpenMP][ Reusable OpenMP context/traits handling to [OpenMP][Part 2] Use reusable OpenMP context/traits handling.
Sat, Jan 11, 8:14 PM · Restricted Project, Restricted Project
jdoerfert retitled D71830: [OpenMP][Part 2] Use reusable OpenMP context/traits handling from [OpenMP] Reusable OpenMP context/traits handling to [OpenMP][ Reusable OpenMP context/traits handling.
Sat, Jan 11, 8:14 PM · Restricted Project, Restricted Project
jdoerfert added a parent revision for D71830: [OpenMP][Part 2] Use reusable OpenMP context/traits handling: D71847: [OpenMP][Part 1] Reusable OpenMP context/traits handling.
Sat, Jan 11, 8:05 PM · Restricted Project, Restricted Project
jdoerfert added a child revision for D71847: [OpenMP][Part 1] Reusable OpenMP context/traits handling: D71830: [OpenMP][Part 2] Use reusable OpenMP context/traits handling.
Sat, Jan 11, 8:05 PM · Restricted Project
jdoerfert added a comment to D72562: [Attributor][Fix] AAHeapToStack and AAIsDead connection.

I see where you are going and we need this, however I just fiddled around with the entire isAssumedDead stuff to make the helper methods aware of dead uses (and instructions). I will see how we can combine this best with H2S and report here, OK?

Sat, Jan 11, 3:04 PM · Restricted Project
jdoerfert added a comment to D71847: [OpenMP][Part 1] Reusable OpenMP context/traits handling.

@JonChesterfield Ping.

Sat, Jan 11, 11:47 AM · Restricted Project
jdoerfert added a comment to D72475: [WIP] Build assume from call.

Only minor points are left. including:
Can we make string attributes and function scope attributes optional (under a command line flag)? Especially the uses created for the latter could easily confuse the call graph and similar passes. That said, we need to teach them to ignore llvm.assume uses (see below).

Sat, Jan 11, 11:47 AM · Restricted Project
jdoerfert added a comment to D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder..

I added quite a bunch of inline comments again :( and some high level stuff below.

Sat, Jan 11, 12:06 AM · Restricted Project, Restricted Project
jdoerfert added inline comments to D72542: [Flang][mlir] add a band-aid to support the creation of mutually recursive types when lowering to LLVM IR.
Sat, Jan 11, 12:06 AM · Restricted Project, Restricted Project
jdoerfert accepted D71914: [OpenMP][Tool] Make tests for archer dependent on TSan.

LGTM.

Sat, Jan 11, 12:06 AM · Restricted Project

Fri, Jan 10

jdoerfert added a comment to D72525: [LIBOMPTARGET] Do not increment/decrement the refcount for "declare target" objects.

I'm generally fine with this. To avoid future problems we could make the RefCount private and introduce modifier methods that do the CONSIDERED_INF check always.

Fri, Jan 10, 11:20 PM · Restricted Project
jdoerfert added a comment to D71617: [WIP][NFC][Attributor] noalias attribute deduction fixme.

Thanks for reviewing further.

  • Removed the Dependence flag.
  • For the conditions to return 'true', in Pred: I observe that the correct condition should be below. This condition means that callsite_argument should not have nocapture and getCtxI() should be reachable from user. I think this, because for noalias propagation to happen w.r.t. (ii) condition, we should get true from checkForAllUses and Pred as well.

    ` // Doesn't have nocapture but reachable. if (!NoCaptureAA.isAssumedNoCapture() && ReachabilityAA.isAssumedReachable(UserI, getCtxI())) return true;

    `
Fri, Jan 10, 11:20 PM · Restricted Project

Thu, Jan 9

jdoerfert added inline comments to D72475: [WIP] Build assume from call.
Thu, Jan 9, 3:24 PM · Restricted Project
jdoerfert added inline comments to D72475: [WIP] Build assume from call.
Thu, Jan 9, 1:50 PM · Restricted Project
jdoerfert added a comment to D72475: [WIP] Build assume from call.

Some high-level comments, haven't gone through it in detail

Thu, Jan 9, 12:54 PM · Restricted Project
jdoerfert accepted D72472: [LIBOMPTARGET]Ignore empty target descriptors..

LGTM.

Thu, Jan 9, 12:35 PM · Restricted Project
jdoerfert added a comment to D72455: [NFC] Refactor TableGen for attributes.

I think this is fine but I'll wait another day for input before I accept.

Thu, Jan 9, 8:59 AM · Restricted Project
jdoerfert added inline comments to D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder..
Thu, Jan 9, 8:59 AM · Restricted Project, Restricted Project
jdoerfert added a comment to D71617: [WIP][NFC][Attributor] noalias attribute deduction fixme.

I found two minor problems that might solve the test issues we see.

Thu, Jan 9, 8:58 AM · Restricted Project
jdoerfert added inline comments to D72400: [MLIR] Add OpenMP dialect with barrier operation.
Thu, Jan 9, 8:40 AM · Restricted Project
jdoerfert resigned from D71770: [lldb] Don't process symlinks deep inside DWARFUnit.
Thu, Jan 9, 8:40 AM · Restricted Project
jdoerfert added inline comments to D71128: [NVPTX][FIX] Expand atomics we cannot handle natively in the ISA.
Thu, Jan 9, 8:40 AM · Restricted Project
jdoerfert requested changes to D72382: [ArgPromotion] Extend search for SafeToUnconditionallyLoad indices to the blocks that must be executed upon entry into the function..

Using post-dominator information is not safe. Take this code:

Thu, Jan 9, 8:30 AM · Restricted Project

Wed, Jan 8

jdoerfert added inline comments to D72025: [PM][CG-SCC] Add a helper to update the call graph from SCC passes.
Wed, Jan 8, 8:24 PM · Restricted Project
jdoerfert added a comment to D72416: [Flang] add flang as a new subproject in cmake.

It helps if you use tags in the title of commits (and reviews), e.g., [Flang] .... Maybe we can make the commit message a bit more specific: "[Flang] Add Flang as subproject in cmake"

Wed, Jan 8, 6:33 PM · Restricted Project, Restricted Project
jdoerfert committed rGa4088c75cc10: [Attributor][FIX] Carefully change invokes to calls (after manifest) (authored by jdoerfert).
[Attributor][FIX] Carefully change invokes to calls (after manifest)
Wed, Jan 8, 5:38 PM
jdoerfert committed rG1e46eb74be65: [Attributor][FIX] Avoid dangling value pointers during code modification (authored by jdoerfert).
[Attributor][FIX] Avoid dangling value pointers during code modification
Wed, Jan 8, 5:38 PM
jdoerfert added inline comments to D71617: [WIP][NFC][Attributor] noalias attribute deduction fixme.
Wed, Jan 8, 11:36 AM · Restricted Project
jdoerfert added inline comments to D72400: [MLIR] Add OpenMP dialect with barrier operation.
Wed, Jan 8, 11:36 AM · Restricted Project
jdoerfert resigned from D69585: PerformPendingInstatiations() already in the PCH.
Wed, Jan 8, 11:36 AM · Restricted Project
jdoerfert requested changes to D71620: [Attributor] AAValueConstantRange: Value range analysis using constant range.

I'll push some fixes that should address the issues people saw (I hope) but there is at least one bug that the test suite exposed.

Wed, Jan 8, 11:27 AM · Restricted Project
jdoerfert added a comment to D71617: [WIP][NFC][Attributor] noalias attribute deduction fixme.

Sorry for my delay. I hope these are the last problems.

Wed, Jan 8, 8:46 AM · Restricted Project
jdoerfert resigned from D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation.
Wed, Jan 8, 8:28 AM · Restricted Project, Restricted Project
jdoerfert resigned from D70720: [llvm-objdump] Display locations of variables alongside disassembly.
Wed, Jan 8, 8:28 AM · debug-info, Restricted Project
jdoerfert added inline comments to D72400: [MLIR] Add OpenMP dialect with barrier operation.
Wed, Jan 8, 7:59 AM · Restricted Project
jdoerfert added inline comments to D71384: [libomptarget] Implement hsa plugin.
Wed, Jan 8, 7:40 AM · Restricted Project

Tue, Jan 7

jdoerfert added inline comments to D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder..
Tue, Jan 7, 3:14 PM · Restricted Project, Restricted Project
jdoerfert updated the diff for D70767: [Attributor] Add an Attributor CG-SCC pass.

Update tests and address comment

Tue, Jan 7, 8:39 AM · Restricted Project
jdoerfert resigned from D72331: OpaquePtr: add type to inalloca attribute..
Tue, Jan 7, 8:02 AM · Restricted Project, Restricted Project

Mon, Jan 6

jdoerfert added a comment to D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder..

Some initial comments. Can we split it in two patches (master/critical)?

Mon, Jan 6, 9:55 PM · Restricted Project, Restricted Project
jdoerfert added inline comments to D72025: [PM][CG-SCC] Add a helper to update the call graph from SCC passes.
Mon, Jan 6, 3:41 PM · Restricted Project
jdoerfert updated the diff for D72025: [PM][CG-SCC] Add a helper to update the call graph from SCC passes.

Addressed comments

Mon, Jan 6, 3:41 PM · Restricted Project
jdoerfert updated the diff for D70767: [Attributor] Add an Attributor CG-SCC pass.

Rebase and better use of D72025

Mon, Jan 6, 3:31 PM · Restricted Project
jdoerfert updated the diff for D72025: [PM][CG-SCC] Add a helper to update the call graph from SCC passes.

First add new edges before the SCC might get split up

Mon, Jan 6, 3:31 PM · Restricted Project
jdoerfert updated the diff for D70927: Introduce a CallGraph updater helper class.

More tests, late function removal and other improvements

Mon, Jan 6, 3:31 PM · Restricted Project
jdoerfert accepted D72287: [OpenMP] Fix incorrect property of __has_attribute() macro.

LGTM.

Mon, Jan 6, 10:35 AM · Restricted Project
jdoerfert added a reviewer for D71710: [instrinsics] Add @llvm.memcpy.inline instrinsics: efriedma.
Mon, Jan 6, 7:57 AM · Restricted Project

Fri, Jan 3

jdoerfert added a comment to D71620: [Attributor] AAValueConstantRange: Value range analysis using constant range.

It seems the failure occurs when old pm is used.
I couldn't find what is causing this error.

@jdoerfert Could you take a look at this?

Fri, Jan 3, 12:10 PM · Restricted Project
jdoerfert accepted D72171: [OpenMP] NFC: Fix trivial typos in comments.

LGTM.

Fri, Jan 3, 11:40 AM · Restricted Project
jdoerfert committed rGd2d2fb19f7ea: [Attributor][FIX] Allow dead users of rewritten function (authored by jdoerfert).
[Attributor][FIX] Allow dead users of rewritten function
Fri, Jan 3, 8:50 AM
jdoerfert committed rG6b9ee2d6cd9f: [Attributor][NFC] Unify the way we delete dead functions (authored by jdoerfert).
[Attributor][NFC] Unify the way we delete dead functions
Fri, Jan 3, 8:50 AM
jdoerfert committed rGc90681b681a7: [Attributor][FIX] Don't crash on ptr2int/int2ptr instructions (authored by jdoerfert).
[Attributor][FIX] Don't crash on ptr2int/int2ptr instructions
Fri, Jan 3, 8:50 AM
jdoerfert committed rG412a0101a99e: [Attributor][FIX] Do not derive nonnull and dereferenceable w/o access (authored by jdoerfert).
[Attributor][FIX] Do not derive nonnull and dereferenceable w/o access
Fri, Jan 3, 8:50 AM
jdoerfert committed rGa4b3588ba2c3: [Attributor][FIX] Return CHANGED once a pessimistic fixpoint is reached. (authored by jdoerfert).
[Attributor][FIX] Return CHANGED once a pessimistic fixpoint is reached.
Fri, Jan 3, 8:49 AM

Thu, Jan 2

jdoerfert added inline comments to D71960: [Attributor] AAUndefinedBehavior: AAValueSimplify in memory accessing instructions..
Thu, Jan 2, 9:58 AM · Restricted Project
jdoerfert added a reviewer for D72058: [OpenMP] Enabling CPU affinity on Darwin platform proposal: protze.joachim.

I can't test this but it looks reasonable, @protze.joachim what do you think?

Thu, Jan 2, 9:49 AM · Restricted Project

Tue, Dec 31

jdoerfert added inline comments to D71974: [Attributor][WIP] Connect AAIsDead with AAUndefinedBehavior.
Tue, Dec 31, 1:37 PM · Restricted Project
jdoerfert added inline comments to D71974: [Attributor][WIP] Connect AAIsDead with AAUndefinedBehavior.
Tue, Dec 31, 10:40 AM · Restricted Project
jdoerfert added inline comments to D71974: [Attributor][WIP] Connect AAIsDead with AAUndefinedBehavior.
Tue, Dec 31, 9:31 AM · Restricted Project
jdoerfert added a comment to D71988: [OpenMP][WIP] Make the kmp_depend_info type fit in 128 bits..

If this really brings a benefit, we need new entry functions and update the compiler(s).

We go to great lengths to align to cache lines in some structs, e.g., kmp_depnode, but we are literally wasting space and alignment for others. Even if we do not adopt this, we should start to adopt some best practices.

There's a difference though: kmp_depend_info_t is passed by the compiler and evaluated once by the runtime library. If I recall correctly, all information is stored in internal structures afterwards (the mentioned kmp_depnode being one of them). Thus I wonder if this change brings a measurable improvement when reading the data once is not on the critical path. Breaking the ABI on the benefit of saving a few bytes on the stack seems a bit overkill just for the sake of doing it.

Tue, Dec 31, 9:12 AM · Restricted Project
jdoerfert committed rGdf3b56c90544: [Attributor][Fix] Avoid leaking memory after D68765 (authored by jdoerfert).
[Attributor][Fix] Avoid leaking memory after D68765
Tue, Dec 31, 9:03 AM
jdoerfert added a comment to D69701: [Utils] Allow "on-the-fly" argument changes for update_test_check scripts.

If I understand this correctly existing check lines between off/on directives should be copied verbatim. Could you add one (that does not match the auto-generated content) to the disabled part of the test?

Tue, Dec 31, 1:20 AM · Restricted Project
jdoerfert committed rGa6c59e0792ed: [Utils] Deal with occasionally deleted functions (authored by jdoerfert).
[Utils] Deal with occasionally deleted functions
Tue, Dec 31, 12:44 AM
jdoerfert closed D68850: [Utils] Deal with occasionally deleted functions.
Tue, Dec 31, 12:43 AM · Restricted Project