Page MenuHomePhabricator

reames (Philip Reames)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 23 2013, 6:32 PM (268 w, 2 h)

Recent Activity

Today

reames committed rL348997: [test] Add a set of test for constant folding deopt operands with CVP.
[test] Add a set of test for constant folding deopt operands with CVP
Wed, Dec 12, 4:57 PM

Mon, Dec 10

reames added a comment to rL348114: [ARM][MC] Move information about variadic register defs into tablegen.

This looks like a generally good improvement, but is it really the case that if any variable operand is a def that all must be? It would seem more general to have a list of variable operands defs and a list of variable operand uses. I have an alternate use case which would require that at some point, but I'm wondering about the current ARM backend use case at the current time.

Mon, Dec 10, 1:06 PM

Tue, Dec 4

reames accepted D51207: Introduce llvm.experimental.widenable_condition intrinsic.

LGTM w/minor comment to be addressed before landing.

Tue, Dec 4, 8:53 PM

Mon, Dec 3

reames added a comment to D55216: [bugpoint][PR29027] Reduce function attributes.

Looks like a good step in the right direction after some cleanup. Any chance you're interested in doing the same for argument or return attributes?

Mon, Dec 3, 3:49 PM

Sun, Dec 2

reames added a comment to D55147: Exclude non-integral pointers in isBytewiseValue.

This patch made me curious because I remembered fixing this one. Turned out that's still a patch we're carrying downstream. Oops. :(

Sun, Dec 2, 5:14 PM

Thu, Nov 29

reames accepted D53889: [CodeGen] Prefer static frame index for STATEPOINT liveness args.

LGTM in the current form. The alternate framing might also be good, but we can come back and undo this later if that works out.

Thu, Nov 29, 2:57 PM

Wed, Nov 28

reames added a comment to D53889: [CodeGen] Prefer static frame index for STATEPOINT liveness args.

In addition to the detailed implementation comment below, I thought of a possible semantic problem. Per the ABI, who "owns" the memory for the arguments? Does either the callee or caller assume it's immutable? If so, then your optimization needs to be restricted to a non-relocating collector since otherwise the collector might be updating a memory location assumed by AA to be immutable and bad things could happen...

Wed, Nov 28, 2:26 PM
reames added inline comments to D53889: [CodeGen] Prefer static frame index for STATEPOINT liveness args.
Wed, Nov 28, 10:04 AM

Sun, Nov 25

reames requested changes to D53889: [CodeGen] Prefer static frame index for STATEPOINT liveness args.
Sun, Nov 25, 7:12 PM
reames accepted D53603: [CodeGen] Take SPAdj into account for STATEPOINT liveness args.

LGTM. I'm actually a bit uncertain of the semantics of this change, but I doubt we're going to find a better reviewer, so I did my best. :)

Sun, Nov 25, 7:09 PM
reames accepted D53892: [CodeGen] Support custom format of stack maps.

We can continue the discussion around finalizeModule and emitStackMaps separately. It's definitely not a blocker for this patch.

Sun, Nov 25, 7:08 PM

Nov 12 2018

reames committed rL346712: [GC][NFC] Simplify code now that we only have one safepoint kind.
[GC][NFC] Simplify code now that we only have one safepoint kind
Nov 12 2018, 2:07 PM
reames committed rL346702: [GC docs] Update the gcroot documentation to reflect recent simplifcations to….
[GC docs] Update the gcroot documentation to reflect recent simplifcations to…
Nov 12 2018, 12:33 PM
reames committed rL346701: [GC] Remove so called PreCall safepoints.
[GC] Remove so called PreCall safepoints
Nov 12 2018, 12:18 PM
reames committed rL346698: [GC][InstCombine] Fix a potential iteration issue.
[GC][InstCombine] Fix a potential iteration issue
Nov 12 2018, 12:03 PM

Nov 11 2018

reames committed rL346632: [GC] Remove unused configuration variable.
[GC] Remove unused configuration variable
Nov 11 2018, 6:37 PM
reames committed rL346631: [GC] Minor style modernization.
[GC] Minor style modernization
Nov 11 2018, 6:29 PM
reames committed rL346621: [GCRoot] Remove some unneccessary complexity.
[GCRoot] Remove some unneccessary complexity
Nov 11 2018, 1:15 PM

Nov 10 2018

reames committed rL346588: [GC] Rename a header for consistency.
[GC] Rename a header for consistency
Nov 10 2018, 8:10 AM

Nov 9 2018

reames added a comment to D53892: [CodeGen] Support custom format of stack maps.

A few drive by comments for the moment.

Nov 9 2018, 4:36 PM
reames committed rL346569: [GC] Simplify linking of GC builtin GC strategies.
[GC] Simplify linking of GC builtin GC strategies
Nov 9 2018, 3:59 PM
reames committed rL346518: [docs][statepoints] Reformulate open issues list.
[docs][statepoints] Reformulate open issues list
Nov 9 2018, 9:12 AM
reames committed rL346513: [docs][statepoint] Expand a bit on problems with mixing references and raw….
[docs][statepoint] Expand a bit on problems with mixing references and raw…
Nov 9 2018, 8:42 AM
reames committed rL346509: [docs][statepoint] tweak a title.
[docs][statepoint] tweak a title
Nov 9 2018, 8:30 AM

Nov 8 2018

reames committed rL346448: [docs][statepoint] Document explicitly provided stack slots.
[docs][statepoint] Document explicitly provided stack slots
Nov 8 2018, 3:25 PM
reames committed rL346447: [docs][statepoints] add a section spelling out simplifications for non….
[docs][statepoints] add a section spelling out simplifications for non…
Nov 8 2018, 3:09 PM
reames committed rL346446: [docs] Add some subsections to make it possible to find portions of the….
[docs] Add some subsections to make it possible to find portions of the…
Nov 8 2018, 2:59 PM
reames committed rL346416: [docs] Clarify ELF section naming for StackMaps and fix a typo.
[docs] Clarify ELF section naming for StackMaps and fix a typo
Nov 8 2018, 9:23 AM
reames committed rL346405: [docs] Clarify expectations for stack map sections and AOT compilers.
[docs] Clarify expectations for stack map sections and AOT compilers
Nov 8 2018, 7:21 AM

Nov 2 2018

reames added a comment to D54021: [LoopSimplifyCFG] Teach LoopSimplifyCFG to constant-fold branches and switches.

Drive by comments.

Nov 2 2018, 5:36 PM

Oct 29 2018

reames added a comment to D53786: [AliasSetTracker] Actually delete instructions from the AliasSetTracker..

This feels like possibly a solution in search of a problem?

One potentially problematic scenario in the current code would be if someone recursively deleted a user instruction inside an AST iteration loop. I think that would delete the pointer value, and if that was the last value in a given AS, invalidate the iteration.

Oct 29 2018, 3:40 PM
reames added a comment to D53786: [AliasSetTracker] Actually delete instructions from the AliasSetTracker..

Hm, can I ask what problem you were originally trying to solve? From a quick search, this code is only invoked through the value handle deleted callback. In which case, the value being deleted is the pointer itself, not the original using instruction.

Oct 29 2018, 3:39 PM
reames accepted D53836: [AliasSetTracker] Cleanup addPointer interface. [NFCI].

p.s. This is an obvious NFC cleanup You'd be fine to land this without review.

Oct 29 2018, 3:24 PM

Oct 7 2018

reames added inline comments to D44748: Track whether the size of a MemoryLocation is precise.
Oct 7 2018, 8:21 PM
reames added a comment to D52827: [LICM] Make LICM able to hoist phis.

Starting with only high level design comments....

Oct 7 2018, 8:15 PM

Oct 4 2018

reames accepted D52760: [SimplifyCFG] Change recursive calls to llvm::SimplifyCFG to instead use an outer while loop to revisit..

LGTM

Oct 4 2018, 10:19 AM
reames accepted D44748: Track whether the size of a MemoryLocation is precise.

LGTM w/one optional, but strongly recommended change.

Oct 4 2018, 2:08 AM

Oct 3 2018

reames added a comment to D52760: [SimplifyCFG] Change recursive calls to llvm::SimplifyCFG to instead use an outer while loop to revisit..

I definitely agree that this recursion is not the ideal way to implement this. I haven't looked closely to see if we always do this recursion or if we sometimes just return without recursing. Any issue with this patch in the short term?

I'm not actively objecting, but I'm not sure I'd bother to pursue this intermediate state as opposed to directly addressing the root issue. If you really want to pursue this approach, I'm willing to review and signoff.

Oct 3 2018, 2:04 AM

Oct 2 2018

reames requested changes to D44748: Track whether the size of a MemoryLocation is precise.
Oct 2 2018, 6:36 AM
reames added a comment to D52760: [SimplifyCFG] Change recursive calls to llvm::SimplifyCFG to instead use an outer while loop to revisit..

Instead of doing the simplification recursively, wouldn't it be better to just iterate externally? That is, what if we split run into an outer function which called an inner helper routine as long as that inner routine was making progress? Or is there a concern that SimplifyCFG does not stabilize to a fixed point?

Oct 2 2018, 6:29 AM

Sep 19 2018

reames requested changes to D44748: Track whether the size of a MemoryLocation is precise.

Mostly minor comments. I considered giving a conditional LGTM, but decided it warranted one last round. I think this is very close to landing.

Sep 19 2018, 3:58 PM

Sep 17 2018

reames added a comment to D52192: Add !thread.private metadata .

Just noting a couple of known gaps (to save reviewers time):

  • No verifier support (will add before submission)
  • Need to add references from each support instruction type in the docs (done to avoid rebase headache)
Sep 17 2018, 2:37 PM
reames retitled D52192: Add !thread.private metadata from Add !thread.private metdata to Add !thread.private metadata .
Sep 17 2018, 2:30 PM
reames created D52192: Add !thread.private metadata .
Sep 17 2018, 2:30 PM

Sep 10 2018

reames committed rL341892: [LICM] (re-)simplify code using MemoryLocation API [NFC].
[LICM] (re-)simplify code using MemoryLocation API [NFC]
Sep 10 2018, 8:29 PM
reames created D51907: [WIP][LICM] Hoist argmemonly calls which can write through some arguments.
Sep 10 2018, 8:16 PM
reames added inline comments to D51895: Fix issues required to remove memset/memcpy/memmove special casing from AliasSetTracker.
Sep 10 2018, 4:16 PM
reames committed rL341880: [AST] Add test coverage of memsets.
[AST] Add test coverage of memsets
Sep 10 2018, 4:15 PM
reames created D51895: Fix issues required to remove memset/memcpy/memmove special casing from AliasSetTracker.
Sep 10 2018, 3:53 PM
reames requested changes to D50377: [LICM] Use ICFLoopSafetyInfo in LICM.

Another round of bugs found.

Sep 10 2018, 2:01 PM
reames accepted D51207: Introduce llvm.experimental.widenable_condition intrinsic.

I'm approving this in the current form, despite a bit of hesitation doing so. I'd like to see the conversation around the restriction of widening based on target block to continue. I think there's a good change we'll want to tweak the semantics there, but I see that as a minor tweak, not a major redesign.

Sep 10 2018, 1:46 PM
reames committed rL341841: [AST] Visit memtransfer arguments in order.
[AST] Visit memtransfer arguments in order
Sep 10 2018, 9:01 AM

Sep 7 2018

reames committed rL341713: [AST] Generalize argument specific aliasing.
[AST] Generalize argument specific aliasing
Sep 7 2018, 2:40 PM
reames closed D50730: [AST] Generalize argument specific aliasing.
Sep 7 2018, 2:40 PM
reames added inline comments to D50730: [AST] Generalize argument specific aliasing.
Sep 7 2018, 10:33 AM
reames accepted D51715: [LICM] Avoid duplicate work during building AliasSetTracker.

Looked at the history in more detail, and decided I'm comfortable with this fix. So, LGTM.

Sep 7 2018, 10:30 AM

Sep 6 2018

reames added inline comments to D51715: [LICM] Avoid duplicate work during building AliasSetTracker.
Sep 6 2018, 4:38 PM
reames added a comment to D51715: [LICM] Avoid duplicate work during building AliasSetTracker.

This looks correct to me, but this is so glaringly bad I'd like a second set of eyes to make sure I'm not missing something. Can someone else confirm?

Sep 6 2018, 4:30 PM

Sep 5 2018

reames requested changes to D51207: Introduce llvm.experimental.widenable_condition intrinsic.

Overall, looks pretty good. One rounds of comments, but I'm expecting this to converge quickly.

Sep 5 2018, 7:19 PM
reames added a comment to D51523: Return "[NFC] Add severe validation of InstructionPrecedenceTracking".

LGTM w/ changes made before submit.

Sep 5 2018, 12:36 PM

Aug 30 2018

reames requested changes to D51523: Return "[NFC] Add severe validation of InstructionPrecedenceTracking".
Aug 30 2018, 9:38 PM

Aug 29 2018

reames updated the diff for D51471: [SimplifyCFG] Reduce memory churn during common case for branch commoning [NFC].

reverse last rebase, the small addition was buggy (iterator invalidation)

Aug 29 2018, 5:45 PM
reames updated the diff for D51471: [SimplifyCFG] Reduce memory churn during common case for branch commoning [NFC].

(slightly modified patch to reduce overhead for multiple predecessor case too)

Aug 29 2018, 5:41 PM
reames created D51471: [SimplifyCFG] Reduce memory churn during common case for branch commoning [NFC].
Aug 29 2018, 5:34 PM
reames committed rL341004: [SimplifyCFG] Rename a variable for readibility of a future change [NFC].
[SimplifyCFG] Rename a variable for readibility of a future change [NFC]
Aug 29 2018, 5:13 PM
reames committed rL341001: [SimplifyCFG] Fix a cost modeling oversight in branch commoning.
[SimplifyCFG] Fix a cost modeling oversight in branch commoning
Aug 29 2018, 5:06 PM
reames committed rL340997: [SimplifyCFG] Common debug handling [NFC].
[SimplifyCFG] Common debug handling [NFC]
Aug 29 2018, 4:23 PM
reames updated the diff for D50730: [AST] Generalize argument specific aliasing.

rebase for minor issue I commented on, also ping?

Aug 29 2018, 3:39 PM
reames committed rL340978: Add a todo and tests to Address a review commnt from D50925 [NFC].
Add a todo and tests to Address a review commnt from D50925 [NFC]
Aug 29 2018, 3:11 PM
reames added inline comments to D50925: [LICM] Hoist stores of invariant values to invariant addresses out of loops.
Aug 29 2018, 3:11 PM
reames committed rL340974: [LICM] Hoist stores of invariant values to invariant addresses out of loops.
[LICM] Hoist stores of invariant values to invariant addresses out of loops
Aug 29 2018, 2:50 PM
reames closed D50925: [LICM] Hoist stores of invariant values to invariant addresses out of loops.
Aug 29 2018, 2:50 PM
reames created D51458: [WIP] Support merging common destinations checks w/uses outside of block.
Aug 29 2018, 2:01 PM

Aug 24 2018

reames abandoned D19559: [LVI] Exploit trivial range information from unknown RHS of icmp.

No longer needed, and extremely stale.

Aug 24 2018, 3:04 PM
reames committed rL340660: [CVP] Extend tests to illustrate an old patch isn't needed.
[CVP] Extend tests to illustrate an old patch isn't needed
Aug 24 2018, 2:57 PM
reames abandoned D7061: Allow PRE to duplicate loads in LICM like loop case.
Aug 24 2018, 2:49 PM
reames abandoned D14263: [LVI] Clarify invariants, common code, and fix latent bugs.
Aug 24 2018, 2:48 PM
reames committed rL340638: [AST] Simplify code minorly using pattern match [NFC].
[AST] Simplify code minorly using pattern match [NFC]
Aug 24 2018, 12:14 PM
reames added inline comments to D50730: [AST] Generalize argument specific aliasing.
Aug 24 2018, 9:34 AM
reames committed rL340617: [LICM] Hoist an invariant_start out of loops if there are no stores executed….
[LICM] Hoist an invariant_start out of loops if there are no stores executed…
Aug 24 2018, 9:25 AM
reames closed D51181: [LICM] Hoist an invariant_start out of loops if there are no stores executed before it.
Aug 24 2018, 9:25 AM
reames abandoned D46212: [WIP][LICM] Hoisting of guards, assumes, and invariant_starts.

All component pieces have been split and landed individually.

Aug 24 2018, 9:18 AM
reames updated the summary of D46212: [WIP][LICM] Hoisting of guards, assumes, and invariant_starts.
Aug 24 2018, 9:16 AM

Aug 23 2018

reames created D51181: [LICM] Hoist an invariant_start out of loops if there are no stores executed before it.
Aug 23 2018, 1:54 PM
reames added a comment to D50925: [LICM] Hoist stores of invariant values to invariant addresses out of loops.

Added example:

(snipped example for length)

Aug 23 2018, 1:03 PM
reames added a comment to D50925: [LICM] Hoist stores of invariant values to invariant addresses out of loops.

There are 3 kinds of tests worth adding:

  1. predicated invariant stores, i.e. the block containing the store itself is predicated and not guaranteed to execute (cannot be handled by LICM)

Covered by existing early exit test.

  1. invariant store value is a phi containing invariant incoming values and the phi result depends on an invariant condition (can be handled by LICM. This patch handles?)

Unclear what you mean here.

  1. invariant store value is a phi containing invariant incoming values and the phi result depends on a variant condition (cannot be handled by LICM safely)

Again, I don't know what you mean by this. If the value is a phi in the loop, it's by definition not invariant.

Aug 23 2018, 12:19 PM
reames updated the diff for D50730: [AST] Generalize argument specific aliasing.

with the right patch this time

Aug 23 2018, 12:14 PM
reames updated the diff for D50730: [AST] Generalize argument specific aliasing.

rebase and incorporate suggestions.

Aug 23 2018, 12:10 PM

Aug 22 2018

reames committed rL340453: [AST] Add a test for attribute intersection.
[AST] Add a test for attribute intersection
Aug 22 2018, 2:11 PM
reames committed rL340443: [AA] Remove a needless variable [NFC].
[AA] Remove a needless variable [NFC]
Aug 22 2018, 12:51 PM
reames committed rL340440: [AST] Minor whitespace cleanup [NFC].
[AST] Minor whitespace cleanup [NFC]
Aug 22 2018, 12:31 PM
reames added inline comments to D50888: [NFC][LICM] Remove too conservative IsMustExecute variable.
Aug 22 2018, 12:28 PM
reames updated the diff for D50925: [LICM] Hoist stores of invariant values to invariant addresses out of loops.

Adding requested tests, ready for review.

Aug 22 2018, 9:21 AM

Aug 21 2018

reames committed rL340384: [AST] Fix a whitespace typo [NFC].
[AST] Fix a whitespace typo [NFC]
Aug 21 2018, 8:37 PM
reames committed rL340383: [AST] Reorder code to reduce a future patch diff [NFC].
[AST] Reorder code to reduce a future patch diff [NFC]
Aug 21 2018, 8:34 PM
reames committed rL340382: [AST] Move a function definition into the cpp [NFC].
[AST] Move a function definition into the cpp [NFC]
Aug 21 2018, 8:33 PM
reames planned changes to D50925: [LICM] Hoist stores of invariant values to invariant addresses out of loops.
Aug 21 2018, 8:30 PM
reames updated the diff for D50925: [LICM] Hoist stores of invariant values to invariant addresses out of loops.

Address first couple comments, another update to come before re-review justified.

Aug 21 2018, 8:30 PM
reames planned changes to D50925: [LICM] Hoist stores of invariant values to invariant addresses out of loops.
Aug 21 2018, 8:21 PM
reames updated the summary of D50925: [LICM] Hoist stores of invariant values to invariant addresses out of loops.
Aug 21 2018, 2:45 PM