Page MenuHomePhabricator

reames (Philip Reames)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 23 2013, 6:32 PM (294 w, 6 d)

Recent Activity

Yesterday

reames added inline comments to D63243: [WIP] Adjust the users of dereferenceable wrt. dereferenceable_globally.
Tue, Jun 18, 8:11 AM · Restricted Project

Mon, Jun 17

reames created D63461: [Util] Add a helper script for converting -print-before-all output into a file based equivelent.
Mon, Jun 17, 3:41 PM · Restricted Project
reames added a comment to D63401: SROA: Allow touching addrspacecast with volatile.

I don't have opinions on this, but Philip might have comments around whether this is okay for GC pointers.

No opinion GC wise, but mostly because I don't understand the intent of this patch. What is the desired outcome here?

For an equivalent bitcast, the type of the underlying alloca is changed into a nicer type and possibly split into multiple allocas (with volatile accesses). I conservatively added checks in r363462 to avoid using the alloca's natural address space for the new operations. I figure it's safer to not change the address space of volatile accesses, but I don't particularly care about this property. InferAddressSpaces avoids changing volatiles, but that was also my idea.

So, just to say this back to you, this would be treating an addrspacecast more like a bitcast when the source is known to be an alloca?

Mon, Jun 17, 2:32 PM
reames added a comment to D62123: [NFC] SimplifyCFG prof branch_weighs handling is simplified.

Again, LGTM.

Mon, Jun 17, 2:23 PM · Restricted Project
reames planned changes to D60783: [LoopPred] Implement a version of the profitability heuristic w/o BPI.

On hold given previous review comments.

Mon, Jun 17, 2:15 PM · Restricted Project
reames abandoned D61922: [IndVarSimplify] Rotate exit comparisons to make them more invariant.

No longer actively pursuing. The idea is reasonable, and only partly overlaps with LFTR, but all of the non-reduced motivating cases are covered by multiple exit LFTR.

Mon, Jun 17, 2:14 PM · Restricted Project
reames added inline comments to D63243: [WIP] Adjust the users of dereferenceable wrt. dereferenceable_globally.
Mon, Jun 17, 2:11 PM · Restricted Project
reames added a comment to D63401: SROA: Allow touching addrspacecast with volatile.

I don't have opinions on this, but Philip might have comments around whether this is okay for GC pointers.

No opinion GC wise, but mostly because I don't understand the intent of this patch. What is the desired outcome here?

Mon, Jun 17, 2:11 PM
reames committed rG44475363e84a: Teach getSCEVAtScope how to handle loop phis w/invariant operands in loops… (authored by reames).
Teach getSCEVAtScope how to handle loop phis w/invariant operands in loops…
Mon, Jun 17, 2:04 PM
reames committed rL363619: Teach getSCEVAtScope how to handle loop phis w/invariant operands in loops….
Teach getSCEVAtScope how to handle loop phis w/invariant operands in loops…
Mon, Jun 17, 2:03 PM
reames closed D63224: Teach getSCEVAtScope how to handle loop phis w/invariant operands in loops w/taken backedges.
Mon, Jun 17, 2:03 PM · Restricted Project
reames committed rGfe8bd96ebd6c: Fix a bug w/inbounds invalidation in LFTR (recommit) (authored by reames).
Fix a bug w/inbounds invalidation in LFTR (recommit)
Mon, Jun 17, 1:30 PM
reames committed rL363613: Fix a bug w/inbounds invalidation in LFTR (recommit).
Fix a bug w/inbounds invalidation in LFTR (recommit)
Mon, Jun 17, 1:29 PM
reames committed rG58c75565f319: Reduced test case for pr42279 in advance of the relevant re-commit + fix (authored by reames).
Reduced test case for pr42279 in advance of the relevant re-commit + fix
Mon, Jun 17, 12:25 PM
reames committed rL363601: Reduced test case for pr42279 in advance of the relevant re-commit + fix.
Reduced test case for pr42279 in advance of the relevant re-commit + fix
Mon, Jun 17, 12:24 PM

Thu, Jun 13

reames accepted D62792: [SimplifyIndVar] Simplify non-overflowing saturating add/sub.

LGTM

Thu, Jun 13, 2:44 PM · Restricted Project
reames committed rG038e01dc9a72: Add a clarifying comment about branching on poison (authored by reames).
Add a clarifying comment about branching on poison
Thu, Jun 13, 12:26 PM
reames committed rL363318: Add a clarifying comment about branching on poison.
Add a clarifying comment about branching on poison
Thu, Jun 13, 12:26 PM
reames committed rGc37be2963421: [LFTR] Rename variable to minimize confusion [NFC] (authored by reames).
[LFTR] Rename variable to minimize confusion [NFC]
Thu, Jun 13, 11:38 AM
reames added inline comments to D62625: LFTR for multiple exit loops.
Thu, Jun 13, 11:38 AM · Restricted Project
reames committed rL363293: [LFTR] Rename variable to minimize confusion [NFC].
[LFTR] Rename variable to minimize confusion [NFC]
Thu, Jun 13, 11:37 AM
reames committed rG42a3fc133d35: [LFTR] Stylistic cleanup as suggested in last review comment of D62939 [NFC] (authored by reames).
[LFTR] Stylistic cleanup as suggested in last review comment of D62939 [NFC]
Thu, Jun 13, 11:33 AM
reames committed rL363292: [LFTR] Stylistic cleanup as suggested in last review comment of D62939 [NFC].
[LFTR] Stylistic cleanup as suggested in last review comment of D62939 [NFC]
Thu, Jun 13, 11:32 AM
reames committed rGeb88badff96d: Fix a bug w/inbounds invalidation in LFTR (authored by reames).
Fix a bug w/inbounds invalidation in LFTR
Thu, Jun 13, 11:21 AM
reames committed rL363289: Fix a bug w/inbounds invalidation in LFTR.
Fix a bug w/inbounds invalidation in LFTR
Thu, Jun 13, 11:20 AM
reames closed D62939: Fix a bug w/inbounds invalidation in LFTR (was: Strengthen LFTR's ability to replace IVs).
Thu, Jun 13, 11:20 AM · Restricted Project
reames added inline comments to D63243: [WIP] Adjust the users of dereferenceable wrt. dereferenceable_globally.
Thu, Jun 13, 9:33 AM · Restricted Project

Wed, Jun 12

reames added a comment to D61652: [Attr] Introduce dereferenceable_globally.

Wow, we have a huge amount of boilerplate when adding an attribute.

Wed, Jun 12, 7:57 PM · Restricted Project
reames added a comment to D61652: [Attr] Introduce dereferenceable_globally.

I'm working on all the remaining changes now and I'll update this patch with the mechanics first before I create a new one to move the users.
We can wait with committing both until we merged the nofree and nosync patches and I updated the isDerefPointer helper function to use them.
Would that be OK?

Absolutely. That's exactly what I'm asking for.

Wed, Jun 12, 5:03 PM · Restricted Project
reames updated the diff for D62625: LFTR for multiple exit loops.

rebase on requested tests

Wed, Jun 12, 4:51 PM · Restricted Project
reames committed rG0bded8442fef: [Tests] Highlight impact of multiple exit LFTR (D62625) as requested by reviewer (authored by reames).
[Tests] Highlight impact of multiple exit LFTR (D62625) as requested by reviewer
Wed, Jun 12, 4:40 PM
reames committed rL363217: [Tests] Highlight impact of multiple exit LFTR (D62625) as requested by reviewer.
[Tests] Highlight impact of multiple exit LFTR (D62625) as requested by reviewer
Wed, Jun 12, 4:36 PM
reames requested changes to D61652: [Attr] Introduce dereferenceable_globally.

Sorry for the late reply. Replying selectively to key points.

Wed, Jun 12, 4:16 PM · Restricted Project
reames accepted D61179: Verifier: check prof branch_weights.

LGTM w/a caveat. I fully expect that landing this would expose lots of existing breakage and would break the build bots. In practice, I don't think you should land this until fixing any issues you've already found. Once that's done, land, but expect to revert and fix found issues for a few cycles.

Wed, Jun 12, 3:31 PM · Restricted Project
reames accepted D62186: [SimplifyCFG] Fix prof branch_weights MD while removing unreachable switch cases.

LGTM w/two required changes.

Wed, Jun 12, 3:27 PM · Restricted Project
reames accepted D62123: [NFC] SimplifyCFG prof branch_weighs handling is simplified.

LGTM w/two required changes. (If you want to just make the changes, you can submit without further review. )

Wed, Jun 12, 3:23 PM · Restricted Project
reames updated the diff for D62939: Fix a bug w/inbounds invalidation in LFTR (was: Strengthen LFTR's ability to replace IVs).

Incorporate Nikita's suggestion about IVs which are already post-increment, and just need their formed changed.

Wed, Jun 12, 3:02 PM · Restricted Project
reames added a comment to D62939: Fix a bug w/inbounds invalidation in LFTR (was: Strengthen LFTR's ability to replace IVs).

If the mustExecuteUBIfPoisonOnPathTo() check is performed, then, as you argue, we do handle S being poison properly. It's just that the check is only performed on pointers, but not integers (in this patch).

Ah, agreed.

Wed, Jun 12, 3:02 PM · Restricted Project
reames committed rGae2581cef3c5: [IndVars] Extend diagnostic -replexitval flag w/ability to bypass hard use… (authored by reames).
[IndVars] Extend diagnostic -replexitval flag w/ability to bypass hard use…
Wed, Jun 12, 12:50 PM
reames committed rL363196: [IndVars] Extend diagnostic -replexitval flag w/ability to bypass hard use….
[IndVars] Extend diagnostic -replexitval flag w/ability to bypass hard use…
Wed, Jun 12, 12:48 PM
reames created D63224: Teach getSCEVAtScope how to handle loop phis w/invariant operands in loops w/taken backedges.
Wed, Jun 12, 12:40 PM · Restricted Project
reames committed rG00e481b75d8a: [Tests] Autogen RLEV test and add tests for a future enhancement (authored by reames).
[Tests] Autogen RLEV test and add tests for a future enhancement
Wed, Jun 12, 12:21 PM
reames committed rL363193: [Tests] Autogen RLEV test and add tests for a future enhancement.
[Tests] Autogen RLEV test and add tests for a future enhancement
Wed, Jun 12, 12:20 PM
reames committed rG851adc000cb2: [Tests] Add tests to highlight sibling loop optimization order issue for exit… (authored by reames).
[Tests] Add tests to highlight sibling loop optimization order issue for exit…
Wed, Jun 12, 12:02 PM
reames committed rL363192: [Tests] Add tests to highlight sibling loop optimization order issue for exit….
[Tests] Add tests to highlight sibling loop optimization order issue for exit…
Wed, Jun 12, 12:01 PM
reames committed rGe51c3d8b8247: [SCEV] Teach computeSCEVAtScope benefit from one-input Phi. PR39673 (authored by reames).
[SCEV] Teach computeSCEVAtScope benefit from one-input Phi. PR39673
Wed, Jun 12, 10:20 AM
reames committed rL363180: [SCEV] Teach computeSCEVAtScope benefit from one-input Phi. PR39673.
[SCEV] Teach computeSCEVAtScope benefit from one-input Phi. PR39673
Wed, Jun 12, 10:20 AM
reames closed D58113: [SCEV] Teach computeSCEVAtScope benefit from one-input Phi. PR39673.
Wed, Jun 12, 10:20 AM · Restricted Project

Tue, Jun 11

reames committed rG02f0b379f563: Fix a bug in getSCEVAtScope w.r.t. non-canonical loops (authored by reames).
Fix a bug in getSCEVAtScope w.r.t. non-canonical loops
Tue, Jun 11, 4:22 PM
reames committed rL363112: Fix a bug in getSCEVAtScope w.r.t. non-canonical loops.
Fix a bug in getSCEVAtScope w.r.t. non-canonical loops
Tue, Jun 11, 4:18 PM
reames added a comment to D61934: [SCEV] Use wrap flags in InsertBinop.

A suggestion on how to make progress here. Introduce the wrap flags to the API, but have *all* callers pass AnyWrap. That must be NFC.

Tue, Jun 11, 3:59 PM · Restricted Project
reames committed rG082cd30327db: Generalize icmp matching in IndVars' eliminateTrunc (authored by reames).
Generalize icmp matching in IndVars' eliminateTrunc
Tue, Jun 11, 3:41 PM
reames committed rL363108: Generalize icmp matching in IndVars' eliminateTrunc.
Generalize icmp matching in IndVars' eliminateTrunc
Tue, Jun 11, 3:40 PM
reames closed D63112: Generalize icmp matching in IndVars' eliminateTrunc.
Tue, Jun 11, 3:40 PM · Restricted Project
reames updated the diff for D62625: LFTR for multiple exit loops.

Rebase on landed prep patches. Ready for actual review, though won't land until after https://reviews.llvm.org/D62939.

Tue, Jun 11, 3:21 PM · Restricted Project
reames added a comment to D62939: Fix a bug w/inbounds invalidation in LFTR (was: Strengthen LFTR's ability to replace IVs).

Let me try to summarize all the problems we can run into and how they will be handled as of this patch. Thankfully, our IV always has the simple form IV = {S,+,1}. Then, there are the following cases:
....

  1. S may be poison. For pointers: Poison/UB check or prevent LFTR. For integers: Not handled. ...

Actually, think 'S' being poisoned is almost fully handled or maybe even fully handled. If 'S' is poison, then IV must be poison on at least the first iteration. Given that, if we find a side effect which must execute on the path to our exit block, then we've proven we must have executed UB. As such, the program is undefined and we have no further obligations.

Tue, Jun 11, 3:08 PM · Restricted Project
reames planned changes to D62860: Aggressively hoist guards out of loops (to provide a target for widening).

Planning a change in direction after offline discussion w/Artur. Instead of framing this as hoisting/widening, I'm going to reframe as a separate loop transform which lifts a trivial guard (by splitting the condition) into the loop preheader where possible. We think this will be easier to test, and may have positive effects beyond guard widening,

Tue, Jun 11, 2:41 PM · Restricted Project

Mon, Jun 10

reames created D63112: Generalize icmp matching in IndVars' eliminateTrunc.
Mon, Jun 10, 5:48 PM · Restricted Project
reames updated the diff for D62939: Fix a bug w/inbounds invalidation in LFTR (was: Strengthen LFTR's ability to replace IVs).

Rebase over tweaked tests and address comments from Nikita.

Mon, Jun 10, 4:26 PM · Restricted Project
reames committed rGefb14f9005dd: [Tests] Adjust LFTR dead-iv tests to bypass undef cases (authored by reames).
[Tests] Adjust LFTR dead-iv tests to bypass undef cases
Mon, Jun 10, 4:15 PM
reames committed rL363002: [Tests] Adjust LFTR dead-iv tests to bypass undef cases.
[Tests] Adjust LFTR dead-iv tests to bypass undef cases
Mon, Jun 10, 4:15 PM
reames updated the diff for D62939: Fix a bug w/inbounds invalidation in LFTR (was: Strengthen LFTR's ability to replace IVs).

First iteration on a combined patch. The tests need updated to reflect the poison/undef split. I'll do that, then rebase on top.

Mon, Jun 10, 4:05 PM · Restricted Project
reames abandoned D62936: Fix a bug w/inbounds invalidation in LFTR.

The issues raised in this and D62939 turned out to be inseparable. Given the meaningful review discussion happened on the other review, I'm abandoning this patch.

Mon, Jun 10, 3:44 PM · Restricted Project
reames updated the diff for D62939: Fix a bug w/inbounds invalidation in LFTR (was: Strengthen LFTR's ability to replace IVs).

Minor rebase. As pointed out by previous comments both from myself and Nikita, the entire approach here is broken. This doesn't actually appear to be separable from D62936. Given all the meaningful review has happened here, I'm going to repurpose this review for the bug fix and close that one.

Mon, Jun 10, 3:43 PM · Restricted Project
reames committed rG1d322ccaacf3: [Tests] Split an LFTR dead-iv case (authored by reames).
[Tests] Split an LFTR dead-iv case
Mon, Jun 10, 3:31 PM
reames committed rL362993: [Tests] Split an LFTR dead-iv case.
[Tests] Split an LFTR dead-iv case
Mon, Jun 10, 3:31 PM
reames accepted D62827: AtomicExpand: Don't crash on non-0 alloca.

LGTM for the AtomicExpand parts. I'm willing to assume you know what you're doing for the AMDGPU bit. :)

Mon, Jun 10, 3:25 PM
reames committed rG4bf1c239908e: Factor out a helper function for readability and reuse in a future patch [NFC] (authored by reames).
Factor out a helper function for readability and reuse in a future patch [NFC]
Mon, Jun 10, 1:41 PM
reames committed rL362980: Factor out a helper function for readability and reuse in a future patch [NFC].
Factor out a helper function for readability and reuse in a future patch [NFC]
Mon, Jun 10, 1:38 PM
reames updated the diff for D62939: Fix a bug w/inbounds invalidation in LFTR (was: Strengthen LFTR's ability to replace IVs).

Rebase on top of newly added tests.

Mon, Jun 10, 12:58 PM · Restricted Project
reames committed rG78c0d7569706: [Tests] Add tests for D62939 (miscompiles around dead pointer IVs) (authored by reames).
[Tests] Add tests for D62939 (miscompiles around dead pointer IVs)
Mon, Jun 10, 12:44 PM
reames committed rL362976: [Tests] Add tests for D62939 (miscompiles around dead pointer IVs).
[Tests] Add tests for D62939 (miscompiles around dead pointer IVs)
Mon, Jun 10, 12:44 PM
reames committed rGa9633d5f0b3d: [LFTR] Use recomputed BE count (authored by reames).
[LFTR] Use recomputed BE count
Mon, Jun 10, 12:17 PM
reames committed rL362975: [LFTR] Use recomputed BE count.
[LFTR] Use recomputed BE count
Mon, Jun 10, 12:16 PM
reames committed rG5d84ccb2303b: Prepare for multi-exit LFTR [NFC] (authored by reames).
Prepare for multi-exit LFTR [NFC]
Mon, Jun 10, 10:49 AM
reames committed rL362971: Prepare for multi-exit LFTR [NFC].
Prepare for multi-exit LFTR [NFC]
Mon, Jun 10, 10:48 AM
reames closed D62880: Prepare for multi-exit LFTR [NFC].
Mon, Jun 10, 10:48 AM · Restricted Project
reames added a comment to D63044: [LangRef] Clarify poison semantics.

Also LGTM. Nice cleanup.

Mon, Jun 10, 10:47 AM · Restricted Project
reames added inline comments to D62939: Fix a bug w/inbounds invalidation in LFTR (was: Strengthen LFTR's ability to replace IVs).
Mon, Jun 10, 10:21 AM · Restricted Project

Thu, Jun 6

reames added inline comments to D62939: Fix a bug w/inbounds invalidation in LFTR (was: Strengthen LFTR's ability to replace IVs).
Thu, Jun 6, 2:37 PM · Restricted Project
reames updated the diff for D62939: Fix a bug w/inbounds invalidation in LFTR (was: Strengthen LFTR's ability to replace IVs).

Remove the buggy phi code Nikita pointed out and otherwise address last round of comments from Nikita.

Thu, Jun 6, 2:37 PM · Restricted Project
reames updated the diff for D62939: Fix a bug w/inbounds invalidation in LFTR (was: Strengthen LFTR's ability to replace IVs).

Remove the two cases Nikita noted.

Thu, Jun 6, 12:20 PM · Restricted Project
reames planned changes to D62939: Fix a bug w/inbounds invalidation in LFTR (was: Strengthen LFTR's ability to replace IVs).
Thu, Jun 6, 12:12 PM · Restricted Project
reames planned changes to D62936: Fix a bug w/inbounds invalidation in LFTR.

Blocked on discussion in D62939 and landing thereof.

Thu, Jun 6, 12:06 PM · Restricted Project
reames committed rG101915cfdaba: [LoopPred] Fix a bug in unconditional latch bailout introduced in r362284 (authored by reames).
[LoopPred] Fix a bug in unconditional latch bailout introduced in r362284
Thu, Jun 6, 11:00 AM
reames committed rL362727: [LoopPred] Fix a bug in unconditional latch bailout introduced in r362284.
[LoopPred] Fix a bug in unconditional latch bailout introduced in r362284
Thu, Jun 6, 10:59 AM
reames added a comment to D61934: [SCEV] Use wrap flags in InsertBinop.

Flags on SCEVs are valid for the current set of users that exist in the loop. If you add *new* users then those flags may no longer be valid. We recently ran into this in LFTR, and this patch seems likely to have the same risk. I have not audited the patch or its output to be sure, just throwing out a possibility.

Thu, Jun 6, 10:33 AM · Restricted Project
reames updated the diff for D62939: Fix a bug w/inbounds invalidation in LFTR (was: Strengthen LFTR's ability to replace IVs).

Revise comments per Nikita's suggestions and some further word smithing of my own.

Thu, Jun 6, 10:22 AM · Restricted Project
reames added a comment to D62939: Fix a bug w/inbounds invalidation in LFTR (was: Strengthen LFTR's ability to replace IVs).

Address a few of Nikita's comment. I ended up heavily revising the structure, so the line locations got a bit confusing.

Thu, Jun 6, 10:16 AM · Restricted Project
reames updated the diff for D62939: Fix a bug w/inbounds invalidation in LFTR (was: Strengthen LFTR's ability to replace IVs).

Cleanup code and fix a couple of bugs in the process.

Thu, Jun 6, 10:08 AM · Restricted Project

Wed, Jun 5

reames created D62939: Fix a bug w/inbounds invalidation in LFTR (was: Strengthen LFTR's ability to replace IVs).
Wed, Jun 5, 7:39 PM · Restricted Project
reames created D62936: Fix a bug w/inbounds invalidation in LFTR.
Wed, Jun 5, 5:57 PM · Restricted Project
reames committed rG13dd125043fa: [Tests] Add poison inference tests for indvars showing both existing transforms… (authored by reames).
[Tests] Add poison inference tests for indvars showing both existing transforms…
Wed, Jun 5, 10:59 AM
reames committed rL362628: [Tests] Add poison inference tests for indvars showing both existing transforms….
[Tests] Add poison inference tests for indvars showing both existing transforms…
Wed, Jun 5, 10:59 AM

Tue, Jun 4

reames planned changes to D62625: LFTR for multiple exit loops.

The NFC portion of this turned out to be a bit more involved than expected. It's been separated into D62880. Once that lands, I'll rebase for the small remaining functional bit. I'm debating just going to all loop exits instead of just the latch exit. On further reflection, I'm not sure it's really going to matter impact wise, and fleshing out problems faster might be worthwhile.

Tue, Jun 4, 2:32 PM · Restricted Project
reames updated the summary of D62880: Prepare for multi-exit LFTR [NFC].
Tue, Jun 4, 2:27 PM · Restricted Project
reames created D62880: Prepare for multi-exit LFTR [NFC].
Tue, Jun 4, 2:26 PM · Restricted Project
reames committed rG0cdaf3a09fea: [Tests] Autogen a test so future changes are visible (authored by reames).
[Tests] Autogen a test so future changes are visible
Tue, Jun 4, 10:29 AM
reames committed rL362532: [Tests] Autogen a test so future changes are visible.
[Tests] Autogen a test so future changes are visible
Tue, Jun 4, 10:29 AM
reames committed rGaf11a4376c11: [Tests] Update a test to consistently use new pass manager and FileCheck the… (authored by reames).
[Tests] Update a test to consistently use new pass manager and FileCheck the…
Tue, Jun 4, 9:20 AM