anna (Anna Thomas)
User

Projects

User does not belong to any projects.

User Details

User Since
Mar 30 2016, 11:13 AM (85 w, 5 d)

Recent Activity

Fri, Nov 17

anna accepted D39954: [IRCE] Smart range intersection.

LGTM. thanks for the comments and clarification.

Fri, Nov 17, 10:33 AM
anna accepted D40168: [IRCE][NFC] Add no wrap flags to no-wrapping SCEV calculation.

lgtm

Fri, Nov 17, 10:32 AM

Thu, Nov 16

anna added inline comments to D39954: [IRCE] Smart range intersection.
Thu, Nov 16, 6:24 AM

Wed, Nov 15

anna added inline comments to D39954: [IRCE] Smart range intersection.
Wed, Nov 15, 6:37 AM

Tue, Nov 14

anna added a comment to D39954: [IRCE] Smart range intersection.

Max, the patch explanation and logic of using SCEV subtraction LGTM. I'll need to take a closer look at your rules and the calculation in subtraction lambda.

Tue, Nov 14, 11:38 AM
anna added inline comments to D39954: [IRCE] Smart range intersection.
Tue, Nov 14, 11:15 AM

Fri, Nov 3

anna abandoned D39598: [LoopPredication] NFC: Refactored code to separate out functions being reused.

Was using this just to verify the diff on phab.

Fri, Nov 3, 7:23 AM
anna created D39598: [LoopPredication] NFC: Refactored code to separate out functions being reused.
Fri, Nov 3, 7:21 AM

Thu, Nov 2

anna added inline comments to D39500: [LoopPredication] Enable predication when latchCheckIV is wider than rangeCheck.
Thu, Nov 2, 2:26 PM
anna updated the diff for D39388: [RS4GC] Strip off invariant.start because memory locations arent invariant.

Updated function names per review comment. NFC wrt previous diff.

Thu, Nov 2, 7:45 AM

Wed, Nov 1

anna created D39500: [LoopPredication] Enable predication when latchCheckIV is wider than rangeCheck.
Wed, Nov 1, 12:03 PM
anna added inline comments to D39388: [RS4GC] Strip off invariant.start because memory locations arent invariant.
Wed, Nov 1, 10:40 AM
anna added a comment to D39388: [RS4GC] Strip off invariant.start because memory locations arent invariant.

just a note: We haven't yet added support to forward loads/stores across calls when the calls are dominated by invariant.start. However, that's a perfectly legal transform, and this patch makes sure we don't miscompile when the transformation is supported.

Wed, Nov 1, 9:29 AM

Tue, Oct 31

anna updated the diff for D39388: [RS4GC] Strip off invariant.start because memory locations arent invariant.

addressed review comments.

Tue, Oct 31, 9:39 AM
anna added a comment to D39388: [RS4GC] Strip off invariant.start because memory locations arent invariant.

Is is possible to also remove the corresponding invariant.end calls to the removed invariant.start calls? This patch will leave orphaned calls to invariant.end sitting around in the IR that have no corresponding start.

Tue, Oct 31, 9:02 AM

Fri, Oct 27

anna created D39388: [RS4GC] Strip off invariant.start because memory locations arent invariant.
Fri, Oct 27, 2:05 PM
anna accepted D39097: [LoopPredication] Handle the case when the guard and the latch IV have different offsets.

LGTM.

Fri, Oct 27, 5:47 AM

Tue, Oct 24

anna requested changes to D39097: [LoopPredication] Handle the case when the guard and the latch IV have different offsets.

Artur, before this patch, we were checking whether RangeCheckIV's postIncExpr is latchCheckIV.

Tue, Oct 24, 7:17 AM

Mon, Oct 23

anna accepted D39082: [IRCE] Smarter detection of empty ranges using SCEV.

LGTM.

Mon, Oct 23, 9:34 AM
anna accepted D38581: [IRCE] Fix intersection between signed and unsigned ranges.

LGTM . Since context is not here but the actual logic was correct and unchanged wrt the previous review, I went by that.

Mon, Oct 23, 9:29 AM

Oct 19 2017

anna added a comment to D39097: [LoopPredication] Handle the case when the guard and the latch IV have different offsets.

@apilipenko please add patch with full context.

Oct 19 2017, 10:16 AM
anna requested changes to D38581: [IRCE] Fix intersection between signed and unsigned ranges.

Some clarification questions and comments inline.

Oct 19 2017, 8:10 AM
anna added inline comments to D39082: [IRCE] Smarter detection of empty ranges using SCEV.
Oct 19 2017, 7:48 AM

Oct 16 2017

anna added a comment to rL315683: [SCEV] Teach SCEV to find maxBECount when loop endbound is variant.

@jdoerfert @grosser
Could we please add as an XFAIL for now, so that it doesn't keep failing the polly buildbots?

I'm actually not working on Polly anymore that's why I tried to get @bollu and @Meinersbur involved.

Oct 16 2017, 8:16 AM
anna added a comment to rL315683: [SCEV] Teach SCEV to find maxBECount when loop endbound is variant.

My local Polly does not pass this test anyway, but I guess Tobias can help you out or maybe @Meinersbur or @bollu. In any case it doesn't seem to be a "loop bounds test" so it should be possible to change it a little bit and keep it around.

Oct 16 2017, 4:52 AM

Oct 13 2017

anna added a comment to rL315683: [SCEV] Teach SCEV to find maxBECount when loop endbound is variant.

What this patch does is, we calculate the MaxBETakenCount for loop with LT terminating condition, when the end bound of loop is varying. We can never know the exact BE Taken count, but the MaxBETakenCount can be computed for loops of form: {Start,+,Stride} LT End.

Oct 13 2017, 2:42 PM
anna added a comment to rL315683: [SCEV] Teach SCEV to find maxBECount when loop endbound is variant.

Thanks @mgrang for pointing the failure. I do see the polly test case and this would kick in because the end bound of the outer loop is variant (bb7 is the latch of outer loop).

Oct 13 2017, 1:34 PM
anna added inline comments to D38825: [SCEV] Teach SCEV to find maxBECount when loop endbound is variant.
Oct 13 2017, 10:23 AM
anna accepted D38849: [RS4GC] Look through vector bitcasts when looking for base pointer.

LGTM.

Oct 13 2017, 8:04 AM

Oct 12 2017

anna added inline comments to D38849: [RS4GC] Look through vector bitcasts when looking for base pointer.
Oct 12 2017, 5:02 PM
anna added a comment to D38766: [CVP] Process binary operations even when def is local.

Did you measure the compile time impact somehow?

Oct 12 2017, 2:09 PM
anna updated the diff for D38825: [SCEV] Teach SCEV to find maxBECount when loop endbound is variant.

Addressed reviewer comments: added more tests regarding overflow, stride negative, stride unknown.
Updated a failing test where we now have more granular max BECount taken
value (This test update was missed in the original diff).

Oct 12 2017, 6:03 AM
anna added inline comments to D38825: [SCEV] Teach SCEV to find maxBECount when loop endbound is variant.
Oct 12 2017, 5:17 AM

Oct 11 2017

anna created D38825: [SCEV] Teach SCEV to find maxBECount when loop endbound is variant.
Oct 11 2017, 2:43 PM
anna edited reviewers for D38766: [CVP] Process binary operations even when def is local, added: reames; removed: philip.
Oct 11 2017, 4:54 AM

Oct 10 2017

anna created D38766: [CVP] Process binary operations even when def is local.
Oct 10 2017, 2:56 PM
anna added inline comments to D38581: [IRCE] Fix intersection between signed and unsigned ranges.
Oct 10 2017, 7:38 AM
anna accepted D38577: [IRCE] Do not process empty safe ranges.

LGTM!

Oct 10 2017, 6:35 AM

Oct 4 2017

anna accepted D38465: Move deleteDeadLoop to LoopUtils. NFC.

LGTM with nits.

Oct 4 2017, 9:31 AM
anna added inline comments to D38465: Move deleteDeadLoop to LoopUtils. NFC.
Oct 4 2017, 7:31 AM

Oct 3 2017

anna added inline comments to D38465: Move deleteDeadLoop to LoopUtils. NFC.
Oct 3 2017, 12:25 PM

Sep 14 2017

anna updated the summary of D35380: [RuntimeUnroll] Add heuristic for unrolling multi-exit loop.
Sep 14 2017, 7:34 AM

Sep 13 2017

anna added a comment to D17080: [LAA] Allow more run-time alias checks by coercing pointer expressions to AddRecExprs.

Hi Silviu,

We only need to convert to an AddRec if we need to emit the runtime checks (see the comment above the original logic for computing NeedRTCheck).

One example would be if we only have reads in the loop. If we convert in hasComputableBounds we end up having unnecessary versioning (at least with regards to the dependence analysis).

Thanks for the clarification. makes sense.

Would the current solution also work in your internal benchmark? If not it might be because something else would need the expression to be an AddRec?

Yes, I tried your patch and it works. I was just curious why we needed all the extra code rather than always checking if we could convert to an AddRec.

Sep 13 2017, 8:03 AM
anna added a comment to D17080: [LAA] Allow more run-time alias checks by coercing pointer expressions to AddRecExprs.

Hi Silviu,
I came across the similar gap, but I aggressively tried converting the PtrSCEV to AddRec in hasComputableBounds, which worked on our internal benchmark. Could you please clarify why you have the Assume to be false in the first call, i.e. how do we know that it's not useful to try and convert PtrSCEV to AddRec?

Sep 13 2017, 7:02 AM

Sep 12 2017

anna updated the diff for D37702: [LV] Clamp the VF to the trip count.

minor review comment: removed unnecessary CHECK.

Sep 12 2017, 7:36 AM
anna added a comment to D37702: [LV] Clamp the VF to the trip count.

@Ayal, any other comments or does this look good to go? Thanks.

Sep 12 2017, 7:26 AM
anna updated the diff for D37702: [LV] Clamp the VF to the trip count.

updated previous diff to CHECK for the correct VF in the first test.

Sep 12 2017, 6:18 AM
anna updated the diff for D37702: [LV] Clamp the VF to the trip count.

Addressed review comments - no longer need the default argument,
updated test to reuse the existing RUN.

Sep 12 2017, 5:01 AM
anna added inline comments to D37702: [LV] Clamp the VF to the trip count.
Sep 12 2017, 4:56 AM

Sep 11 2017

anna created D37702: [LV] Clamp the VF to the trip count.
Sep 11 2017, 11:28 AM
anna added inline comments to D37425: LoopVectorize: MaxVF should not be larger than the loop trip count.
Sep 11 2017, 10:09 AM

Aug 30 2017

anna accepted D36215: [IRCE] Return "Identify loops with latch comparison against current IV value".

LGTM

Aug 30 2017, 6:27 AM

Aug 25 2017

anna added a comment to D36215: [IRCE] Return "Identify loops with latch comparison against current IV value".

Max, could you please update the patch with full context?

Aug 25 2017, 6:18 AM

Aug 18 2017

anna accepted D36873: [IRCE] Fix buggy behavior in Clamp.

LGTM w/comment.

Aug 18 2017, 6:13 AM

Aug 17 2017

anna requested changes to D36215: [IRCE] Return "Identify loops with latch comparison against current IV value".

Comments inline.

Aug 17 2017, 6:42 AM

Aug 9 2017

anna accepted D36509: [IRCE][NFC] Rename IndVarNext to IndVarBase.

LGTM. FWIW, these sort of changes can be checked in without precommit approval.

Aug 9 2017, 12:12 PM

Aug 8 2017

anna updated the diff for D36244: [LoopVectorize] Fix assertion failure in Fcmp vectorization.

Addressed review comments.

Aug 8 2017, 7:46 AM
anna added inline comments to D36244: [LoopVectorize] Fix assertion failure in Fcmp vectorization.
Aug 8 2017, 7:46 AM

Aug 4 2017

anna added inline comments to D36244: [LoopVectorize] Fix assertion failure in Fcmp vectorization.
Aug 4 2017, 6:13 AM

Aug 3 2017

anna accepted D35302: [IRCE] Recognize loops with unsigned latch conditions.

LGTM.

Aug 3 2017, 7:15 AM

Aug 2 2017

anna created D36244: [LoopVectorize] Fix assertion failure in Fcmp vectorization.
Aug 2 2017, 2:45 PM
anna requested changes to D35302: [IRCE] Recognize loops with unsigned latch conditions.

Looks almost ready to go. Some comments regarding additional tests.

Aug 2 2017, 6:50 AM

Jul 31 2017

anna added a comment to D35380: [RuntimeUnroll] Add heuristic for unrolling multi-exit loop.

ping

Jul 31 2017, 6:47 AM

Jul 28 2017

anna added a comment to D33320: [SLP] Improve comments and naming of functions/variables/members, NFC..

We just identified this commit as the cause of a 10x slowdown when compiling shared_sha256.c in the llvm test-suite/CTMark (x86, no special flags should get you the default -O3) showing up on our performance tracking.

Did any of the planned improvements make it to ToT yet?

Seems like this has recovered on ToT.

This was the review thread: https://reviews.llvm.org/D34881

Jul 28 2017, 6:57 PM
anna accepted D34881: [SLP] Allow vectorization of the instruction from the same basic blocks only, NFC..

LGTM.

Jul 28 2017, 12:40 PM
anna added inline comments to D34881: [SLP] Allow vectorization of the instruction from the same basic blocks only, NFC..
Jul 28 2017, 11:19 AM
anna requested changes to D35302: [IRCE] Recognize loops with unsigned latch conditions.

Couple of minor changes. I haven't looked at the tests yet.

Jul 28 2017, 10:07 AM

Jul 27 2017

anna accepted D35973: [JT] Add an option to dump LazyValueInfo after the run.

LGTM. Thanks!

Jul 27 2017, 7:54 PM
anna added a comment to D35840: All libcalls should be considered to be GC-leaf functions..
In D35840#822809, @anna wrote:

This change looks good to me. However, I would suggest separating out the PlaceSafepoints (and it's testcase) change from the RewriteStatepointsForGC change. We don't use placeSafepoint anymore, so we should perhaps just deprecate it upstream, instead of fixing it.
@reames: What do you suggest?

With the codebase in its current state, the PlaceSafepoints & RewriteStatepointsForGC changes cannot be separated. Fundamentally, the change is to behaviour of callsGCLeafFunction() -- making it flag lib calls as gc-leaf. That change required changing the prototype for the function, which in turn required the change in PlaceSafepoints.

The change could be contained to add the libcall check to just RewriteStatepointsForGC in the NeedsRewrite lambda, but that's less of a proper fix in my view -- it fixes a location of the bug's manifestation, but not the source of the bug.

Jul 27 2017, 9:21 AM
anna accepted D35840: All libcalls should be considered to be GC-leaf functions..

This change looks good to me. However, I would suggest separating out the PlaceSafepoints (and it's testcase) change from the RewriteStatepointsForGC change. We don't use placeSafepoint anymore, so we should perhaps just deprecate it upstream, instead of fixing it.
@reames: What do you suggest?

Jul 27 2017, 8:17 AM

Jul 26 2017

anna added a comment to D34769: [X86] X86::CMOV to Branch heuristic based optimization.

We are seeing couple of regressions with this optimization on our internal benchmarks on skylake hardware.
This is fixed when we turn off the optimization using -x86-cmov-converter=false

Jul 26 2017, 1:58 PM

Jul 25 2017

anna accepted D35539: [IRCE] Handle loops with step different from 1/-1.

changes LGTM w/ comments above addressed.

Jul 25 2017, 1:28 PM

Jul 21 2017

anna updated the diff for D35380: [RuntimeUnroll] Add heuristic for unrolling multi-exit loop.

addressed review comments.

Jul 21 2017, 11:17 AM
anna added inline comments to D35380: [RuntimeUnroll] Add heuristic for unrolling multi-exit loop.
Jul 21 2017, 7:45 AM

Jul 19 2017

anna added a comment to D35539: [IRCE] Handle loops with step different from 1/-1.

The change generally looks good to me. I'll take a closer look tomorrow. Some minor comments inline.

Jul 19 2017, 4:53 PM

Jul 13 2017

anna created D35380: [RuntimeUnroll] Add heuristic for unrolling multi-exit loop.
Jul 13 2017, 1:14 PM
anna accepted D35347: [IRCE] Fix corner case with Start = INT_MAX.

LGTM w/ comments inline.

Jul 13 2017, 8:00 AM
anna added inline comments to D35304: [RuntimeLoopUnrolling] Update DomTree correctly when exit blocks have successors.
Jul 13 2017, 5:57 AM

Jul 12 2017

anna added a comment to D35304: [RuntimeLoopUnrolling] Update DomTree correctly when exit blocks have successors.

Hi Michael,

Don't we break critical edges before unrolling? If so, then I think this situation should never happen (and if it does, we need to understand why).

Michael

Jul 12 2017, 2:17 PM
anna added inline comments to D35304: [RuntimeLoopUnrolling] Update DomTree correctly when exit blocks have successors.
Jul 12 2017, 12:53 PM
anna created D35304: [RuntimeLoopUnrolling] Update DomTree correctly when exit blocks have successors.
Jul 12 2017, 6:55 AM

Jul 7 2017

anna added inline comments to D35107: Re-enable "[IndVars] Canonicalize comparisons between non-negative values and indvars".
Jul 7 2017, 6:16 AM
anna accepted D35107: Re-enable "[IndVars] Canonicalize comparisons between non-negative values and indvars".

LGTM.

Jul 7 2017, 5:43 AM

Jul 6 2017

anna updated the diff for D35057: [SafepointIRVerifier] Avoid false positives in GC verifier for compare between pointers.

rebased over the NFC refactoring.

Jul 6 2017, 7:00 PM
anna added a comment to D35057: [SafepointIRVerifier] Avoid false positives in GC verifier for compare between pointers.

Need to rebase code over the NFC refactoring.

Jul 6 2017, 5:43 PM
anna added inline comments to D35057: [SafepointIRVerifier] Avoid false positives in GC verifier for compare between pointers.
Jul 6 2017, 11:11 AM
anna created D35057: [SafepointIRVerifier] Avoid false positives in GC verifier for compare between pointers.
Jul 6 2017, 7:34 AM

Jul 4 2017

anna added inline comments to D15940: Add verifier pass for finding GC relocation bugs.
Jul 4 2017, 6:23 PM
anna accepted D34421: [FastISel][SelectionDAG]Teach fastISel about GC intrinsics.

I'm going ahead and accepting the revision based on @skatkov 's LGTM above (and haven't heard any comments from other reviewers). If there's some future comments, I'll address it in post commit review. Thanks.

Jul 4 2017, 6:57 AM

Jun 30 2017

anna added a comment to D33001: [RuntimeUnrolling] Add logic for loops with multiple exit blocks.

LGTM. Thanks for all the work on this!

Jun 30 2017, 11:48 AM
anna added a comment to D33320: [SLP] Improve comments and naming of functions/variables/members, NFC..

I'm working on the patch that stops vectorization if the parent basic block of the instruction is not BB or the instruction was processed already, but it's quite hard to add a test for this change. I can publish it as NFC, because we just limiting the number of analyzed instructions if this is acceptable.

Jun 30 2017, 9:11 AM
anna added a comment to D33320: [SLP] Improve comments and naming of functions/variables/members, NFC..

@anemet: We saw the degradations with respect to this change (D33320) itself, not before the original change. Here's the code change as I see it. Please correct if this is wrong:

  1. we were initially processing exactly one node in tryToVectorizeHorReductionOrInstOperands equivalent function, before any changes below.
  2. after D25517, we are processing at most 2 ^ 12 nodes in tryToVectorizeHorReductionOrInstOperands, but they are limited to a single basic block.
  3. in this refinement change (D33320), we are processing at most 2 ^ 12 nodes, but they are no longer limited to a single basic block.
Jun 30 2017, 7:52 AM
anna added a comment to D25517: [SLPVectorizer] Improved support of partial tree vectorization..

Have you checked the compile-time impact of this change? I wonder if you need some additional cutoff because it looks like you might be increasing the worst-case complexity of the algorithm here.

Just FYI: this patch was improved at here: https://reviews.llvm.org/D33320
Ongoing discussion about compile time impact is on that review thread. We do see huge compile time impact (internally) because of the change in this function (the processing has increased from a single node to a max of 2 ^ 12 nodes).

Jun 30 2017, 7:10 AM
anna added a comment to D33320: [SLP] Improve comments and naming of functions/variables/members, NFC..
In D33320#795834, @anna wrote:

Hi Alexey, Adam,

From what I can see in this algorithm, there is no limit on the actual size of the stack in the loop. The level variable controls just the recursion limit. So, in effect, IIUC, the max total number of operands being processed by the while loop is 2 ^ RecursionLimit (it's to the base 2 because we avoid phi nodes).

It does not limits the number of processed nodes, it limits the tree height just like it was before.

Yes, but limiting the tree height itself is not enough right? Now, in the worst case, 2^12 nodes being processed in the tryToVectorizeHorReductionOrInstOperands, when earlier it was just a single node (i.e. before this change: https://reviews.llvm.org/D25517).

Jun 30 2017, 7:05 AM
anna added a comment to D34421: [FastISel][SelectionDAG]Teach fastISel about GC intrinsics.

ping2.

Jun 30 2017, 6:40 AM

Jun 29 2017

anna added a comment to D33320: [SLP] Improve comments and naming of functions/variables/members, NFC..

Hi Alexey, Adam,

Jun 29 2017, 2:15 PM
anna updated the summary of D15940: Add verifier pass for finding GC relocation bugs.
Jun 29 2017, 10:46 AM
anna commandeered D15940: Add verifier pass for finding GC relocation bugs.

Taking this over from Philip. Will address the review comments.
Looks like there's conflicting opinion about where to place this - in the IR directory next to verifier, or in the analysis directory.
I prefer keeping it in the IR directory itself if no one has objections.

Jun 29 2017, 10:44 AM
anna edited reviewers for D34421: [FastISel][SelectionDAG]Teach fastISel about GC intrinsics, added: mcrosier, apilipenko; removed: qikon.
Jun 29 2017, 10:34 AM

Jun 28 2017

anna updated the diff for D33001: [RuntimeUnrolling] Add logic for loops with multiple exit blocks.

NFC wrt previous diff: updated the test CHECK statements to avoid using the auto generated CHECK statements.
Manually updating CHECK statements makes the actual test much clearer.

Jun 28 2017, 7:27 AM