Page MenuHomePhabricator

reames (Philip Reames)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Yesterday

reames committed rG78ae870f114d: {tests] Rerun autogen to reduce a diff [nfc] (authored by reames).
{tests] Rerun autogen to reduce a diff [nfc]
Fri, Mar 31, 12:47 PM · Restricted Project, Restricted Project
reames added a comment to D147336: [IVDescriptors] Add pointer InductionDescriptors with non-constant strides (try 2).

Hello. About the performance changes - I have been looking into them a little. A lot of it looks good, especially where gather/scatter are available which this can make use of. There are some cases where it wasn't doing quite as well under some places in MVE, but https://reviews.llvm.org/D147331 should hopefully sort out the largest of them. Sam/Nick should be back on Monday to review it. We have some other cases where gather/scatter are not available for whatever reason and those don't look as good but the changes are much smaller. Again it likely comes down to adjusting the costmodel, perhaps for the cost of the scalarized loads in getMemInstScalarizationCost needs to be a little higher under MVE. I wouldn't consider any of that blockers considering all the improvements.

This is all good news, thanks!

Fri, Mar 31, 10:56 AM · Restricted Project, Restricted Project
reames updated the summary of D147336: [IVDescriptors] Add pointer InductionDescriptors with non-constant strides (try 2).
Fri, Mar 31, 9:32 AM · Restricted Project, Restricted Project
reames added a comment to D147336: [IVDescriptors] Add pointer InductionDescriptors with non-constant strides (try 2).

@dmgreen I'd really appreciate any input you can share on the performance swing you observed.

Fri, Mar 31, 9:32 AM · Restricted Project, Restricted Project
reames added a comment to rG498aa534f472: [IVDescriptors] Add pointer InductionDescriptors with non-constant strides.

Hello - We also saw some of the similar assert above, along with quite a few very large performance changes. Some of those were good but some of them looked pretty bad. I've reverted until the correctness issues can be resolved and am trying to look through the performance - it might well be that the cost model isn't expecting non-constant pointer inductions, so is not getting the correct costs.

I've posted a review with a second attempt here: https://reviews.llvm.org/D147336. This contains the functional fix, but not yet anything on the performance side. Let's continue the discussion on the perf issue there.

Fri, Mar 31, 9:27 AM · Restricted Project, Restricted Project
reames requested review of D147336: [IVDescriptors] Add pointer InductionDescriptors with non-constant strides (try 2).
Fri, Mar 31, 9:25 AM · Restricted Project, Restricted Project
reames committed rGa512ce5e1291: [LV] Add tests for non-constant stride pointer inductions (authored by reames).
[LV] Add tests for non-constant stride pointer inductions
Fri, Mar 31, 9:11 AM · Restricted Project, Restricted Project

Thu, Mar 30

reames updated the diff for D147183: [RISCV][docs] Document which revision of the specification we implement.

Use wording suggested by @asb

Thu, Mar 30, 1:12 PM · Restricted Project, Restricted Project
reames committed rG498aa534f472: [IVDescriptors] Add pointer InductionDescriptors with non-constant strides (authored by reames).
[IVDescriptors] Add pointer InductionDescriptors with non-constant strides
Thu, Mar 30, 11:56 AM · Restricted Project, Restricted Project
reames committed rG1c5bb25d62bb: [RISCV][LV] Add test coverage for strided access patterns [nfc] (authored by reames).
[RISCV][LV] Add test coverage for strided access patterns [nfc]
Thu, Mar 30, 10:02 AM · Restricted Project, Restricted Project

Wed, Mar 29

reames requested review of D147183: [RISCV][docs] Document which revision of the specification we implement.
Wed, Mar 29, 2:02 PM · Restricted Project, Restricted Project
reames added a comment to D147117: [SCEV] When computing trip count, only zext if necessary.

Two levels of comments here.

Wed, Mar 29, 11:39 AM · Restricted Project, Restricted Project
reames accepted D147147: [RISCV] Add codegen for the experimental zicond extension.

LGTM

Wed, Mar 29, 7:39 AM · Restricted Project, Restricted Project
reames committed rG57492b1eeb99: [RISCV] Cost model for general case of dual vector permute (authored by reames).
[RISCV] Cost model for general case of dual vector permute
Wed, Mar 29, 7:37 AM · Restricted Project, Restricted Project
reames closed D147063: [RISCV] Cost model for general case of dual vector permute.
Wed, Mar 29, 7:37 AM · Restricted Project, Restricted Project

Tue, Mar 28

reames requested review of D147063: [RISCV] Cost model for general case of dual vector permute.
Tue, Mar 28, 9:52 AM · Restricted Project, Restricted Project
reames committed rG3e90772b46f3: [RISCV] Add shuffle cost tests for general fixed vector permute [nfc] (authored by reames).
[RISCV] Add shuffle cost tests for general fixed vector permute [nfc]
Tue, Mar 28, 8:59 AM · Restricted Project, Restricted Project
reames committed rG45d00bff43ea: [RISCV] Consolidate and extend fixed vector shuffle cost tests [nfc] (authored by reames).
[RISCV] Consolidate and extend fixed vector shuffle cost tests [nfc]
Tue, Mar 28, 8:59 AM · Restricted Project, Restricted Project
reames committed rG4b7b612c5d34: [RISCV][TTI] Extract getConstantPoolLoadCost helper routine [nfc] (authored by reames).
[RISCV][TTI] Extract getConstantPoolLoadCost helper routine [nfc]
Tue, Mar 28, 7:49 AM · Restricted Project, Restricted Project
reames committed rG64f69e453ed3: [RISCV] Cost model for general case of single vector permute (authored by reames).
[RISCV] Cost model for general case of single vector permute
Tue, Mar 28, 7:34 AM · Restricted Project, Restricted Project
reames closed D147000: [RISCV] Cost model for general case of single vector permute.
Tue, Mar 28, 7:34 AM · Restricted Project, Restricted Project

Mon, Mar 27

reames requested review of D147000: [RISCV] Cost model for general case of single vector permute.
Mon, Mar 27, 1:46 PM · Restricted Project, Restricted Project
reames accepted D146998: [RISCV] Replace an 'else if' with 'else'+assert. NFC.

LGTM

Mon, Mar 27, 1:30 PM · Restricted Project, Restricted Project
reames accepted D146983: [RISCV] Handle non-recursive muls of strides in gather/scatter lowering.

LGTM once Craig's style comment addressed.

Mon, Mar 27, 9:54 AM · Restricted Project, Restricted Project

Fri, Mar 24

reames accepted D146835: [RISCV] Enable usubo formation in CodeGenPrepare..

LGTM w/comment

Fri, Mar 24, 1:32 PM · Restricted Project, Restricted Project
reames accepted D146789: [LegalizeTypes][TargetLowering][RISCV] Fix regressions from D146786..

LGTM

Fri, Mar 24, 11:44 AM · Restricted Project, Restricted Project
reames added inline comments to D146835: [RISCV] Enable usubo formation in CodeGenPrepare..
Fri, Mar 24, 11:41 AM · Restricted Project, Restricted Project
reames added a comment to D146711: [RISCV] Lower insert subvector shuffles as vslideups.

Looking at some code which benefits from this change, I noticed an opportunity for a follow up.

Fri, Mar 24, 11:37 AM · Restricted Project, Restricted Project
reames requested changes to D146208: [ASAN] Support memory checks on vp.load/store..
Fri, Mar 24, 8:57 AM · Restricted Project, Restricted Project
reames accepted D146711: [RISCV] Lower insert subvector shuffles as vslideups.

LGTM w/minor comments.

Fri, Mar 24, 7:53 AM · Restricted Project, Restricted Project

Thu, Mar 23

reames added inline comments to D146711: [RISCV] Lower insert subvector shuffles as vslideups.
Thu, Mar 23, 2:14 PM · Restricted Project, Restricted Project
reames accepted D146747: [RISCV] Model select and insertsubvector shuffle kinds.

LGTM - once all the earlier patches have been approved.

Thu, Mar 23, 2:04 PM · Restricted Project, Restricted Project
reames accepted D146746: [RISCV] Add test cases for cost modelling more shuffle kinds.

LGTM - though can you split this into two patches when you land? The SLP bit deserves a slightly different description focus.

Thu, Mar 23, 2:01 PM · Restricted Project, Restricted Project
reames accepted D146710: [RISCV] Add test for shuffles that could be done as vmerges.

LGTM

Thu, Mar 23, 1:58 PM · Restricted Project, Restricted Project
reames requested changes to D146711: [RISCV] Lower insert subvector shuffles as vslideups.
Thu, Mar 23, 1:56 PM · Restricted Project, Restricted Project
reames added a comment to D146711: [RISCV] Lower insert subvector shuffles as vslideups.

I think you're going one step too far with this patch. Not in the sense that this patch isn't justified - only in the sense you should do a simpler case first.

Thu, Mar 23, 1:56 PM · Restricted Project, Restricted Project
reames committed rG2cfd06ba672f: [BoundsChecking] Don't crash on scalable vector sizes (authored by reames).
[BoundsChecking] Don't crash on scalable vector sizes
Thu, Mar 23, 8:54 AM · Restricted Project, Restricted Project
reames committed rG5eb9acf9be3c: [HWASAN] Instrument scalable load/store without crashing (authored by reames).
[HWASAN] Instrument scalable load/store without crashing
Thu, Mar 23, 8:25 AM · Restricted Project, Restricted Project
reames committed rG5bcb4c4da99c: [MSAN] Support load and stores of scalable vector types (authored by reames).
[MSAN] Support load and stores of scalable vector types
Thu, Mar 23, 7:38 AM · Restricted Project, Restricted Project
reames closed D146157: [MSAN] Support load and stores of scalable vector types.
Thu, Mar 23, 7:38 AM · Restricted Project, Restricted Project

Wed, Mar 22

reames updated the diff for D146157: [MSAN] Support load and stores of scalable vector types.

Address review comment

Wed, Mar 22, 2:10 PM · Restricted Project, Restricted Project
reames added inline comments to D146157: [MSAN] Support load and stores of scalable vector types.
Wed, Mar 22, 2:10 PM · Restricted Project, Restricted Project
reames committed rG473e9adb84c2: [MSAN] Update vector load/store tests to use proper attribute (authored by reames).
[MSAN] Update vector load/store tests to use proper attribute
Wed, Mar 22, 2:03 PM · Restricted Project, Restricted Project
reames committed rG0766c1bd5c0e: [LFTR] Simplify integer case for genLoopLimit [nfc-ish] (authored by reames).
[LFTR] Simplify integer case for genLoopLimit [nfc-ish]
Wed, Mar 22, 12:22 PM · Restricted Project, Restricted Project
reames closed D146638: [LFTR] Simplify integer case for genLoopLimit [nfc-ish].
Wed, Mar 22, 12:22 PM · Restricted Project, Restricted Project
reames committed rG6afcc54ac7d6: [SCEV] Infer no-self-wrap via constant ranges (authored by reames).
[SCEV] Infer no-self-wrap via constant ranges
Wed, Mar 22, 12:07 PM · Restricted Project, Restricted Project
reames closed D146596: [SCEV] Infer no-self-wrap via constant ranges.
Wed, Mar 22, 12:06 PM · Restricted Project, Restricted Project
reames updated the diff for D146596: [SCEV] Infer no-self-wrap via constant ranges.

Address review comment.

Wed, Mar 22, 10:08 AM · Restricted Project, Restricted Project
reames requested review of D146638: [LFTR] Simplify integer case for genLoopLimit [nfc-ish].
Wed, Mar 22, 9:16 AM · Restricted Project, Restricted Project
reames requested changes to D146626: [RISCV] Add test case for combining vrgather.
Wed, Mar 22, 7:55 AM · Restricted Project, Restricted Project
reames retitled D145085: [RISCV] Lower fixed length interleaved accesses via vssegN/vlsegN from [RISCV] Lower interleaved accesses to [RISCV] Lower fixed length interleaved accesses via vssegN/vlsegN.
Wed, Mar 22, 7:47 AM · Restricted Project, Restricted Project
reames accepted D146442: [RISCV][NFC] Make interleaved access test more vectorizable.

p.s. The previous case might be interesting to explore later. It should involve a stride store, but separating that into it's own patch series makes complete sense.

Wed, Mar 22, 7:47 AM · Restricted Project, Restricted Project
reames accepted D146025: [RISCV][NFC] Add more tests for SLP vectorization (binops on load/store).

LGTM

Wed, Mar 22, 7:44 AM · Restricted Project, Restricted Project

Tue, Mar 21

reames committed rG06006f438e1b: [LFTR] Minor style cleanup [nfc] (authored by reames).
[LFTR] Minor style cleanup [nfc]
Tue, Mar 21, 7:19 PM · Restricted Project, Restricted Project
reames committed rG206dc5453477: [LFTR] Use evaluateAtIteration in genLoopLimit [nfc] (authored by reames).
[LFTR] Use evaluateAtIteration in genLoopLimit [nfc]
Tue, Mar 21, 7:19 PM · Restricted Project, Restricted Project
reames committed rGa124b4c7f9f8: [LFTR] Simplify another case under assumption exit counts are integers [nfc] (authored by reames).
[LFTR] Simplify another case under assumption exit counts are integers [nfc]
Tue, Mar 21, 7:19 PM · Restricted Project, Restricted Project
reames closed D146468: [LFTR] Use evaluateAtIteration in genLoopLimit [nfc].
Tue, Mar 21, 7:18 PM · Restricted Project, Restricted Project
reames added inline comments to D146596: [SCEV] Infer no-self-wrap via constant ranges.
Tue, Mar 21, 7:02 PM · Restricted Project, Restricted Project
reames requested review of D146596: [SCEV] Infer no-self-wrap via constant ranges.
Tue, Mar 21, 7:01 PM · Restricted Project, Restricted Project
reames committed rGc49e56a2954f: [SCEV] Add coverage for a missing flag inference case (authored by reames).
[SCEV] Add coverage for a missing flag inference case
Tue, Mar 21, 6:42 PM · Restricted Project, Restricted Project
reames added a comment to D134893: [LSR][TTI][RISCV] Add isAllowTerminatingConditionFoldingAfterLSR into TTI and enable it for RISC-V.

Ok, at this point all the known soundness problems are fixed in tree. I was simply fixing ones obvious on inspection, and have not done any testing of this mechanism beyond the LIT tests themselves.

Tue, Mar 21, 9:52 AM · Restricted Project, Restricted Project
reames added a comment to D146415: [LSR]: Fix cast to BranchInst..

I landed a fix for this in 00fdd2cb6c55744ae21f18c377854e9359273454. This patch can be abandoned.

Tue, Mar 21, 9:36 AM · Restricted Project, Restricted Project
reames committed rG00fdd2cb6c55: [LSR] Don't crash on non-branch terminator in -lsr-term-fold (authored by reames).
[LSR] Don't crash on non-branch terminator in -lsr-term-fold
Tue, Mar 21, 9:36 AM · Restricted Project, Restricted Project
reames committed rGe9df5d62c835: [LSR] Remove a couple stale comments in lsr-term-fold (authored by reames).
[LSR] Remove a couple stale comments in lsr-term-fold
Tue, Mar 21, 9:21 AM · Restricted Project, Restricted Project
reames accepted D146529: [RISCV][NFC] Add test case for SLP reduction vectorization failure.

LGTM

Tue, Mar 21, 8:42 AM · Restricted Project, Restricted Project
reames committed rG53e9a5ddc0a2: [LSR] Fix "new use of poison" problem in lsr-term-fold (authored by reames).
[LSR] Fix "new use of poison" problem in lsr-term-fold
Tue, Mar 21, 8:24 AM · Restricted Project, Restricted Project
reames closed D146464: [LSR] Fix "new use of poison" problem in lsr-term-fold.
Tue, Mar 21, 8:24 AM · Restricted Project, Restricted Project
reames committed rGb33f5e7ed3cd: [LSR] Use evaluateAtIteration in lsr-term-fold (authored by reames).
[LSR] Use evaluateAtIteration in lsr-term-fold
Tue, Mar 21, 8:13 AM · Restricted Project, Restricted Project
reames committed rGb7af34c303ca: [LSR] Add a test case for (another) miscompile in lsr-term-fold (authored by reames).
[LSR] Add a test case for (another) miscompile in lsr-term-fold
Tue, Mar 21, 8:13 AM · Restricted Project, Restricted Project
reames closed D146457: [LSR] Use evaluateAtIteration in lsr-term-fold.
Tue, Mar 21, 8:13 AM · Restricted Project, Restricted Project
reames added a comment to D146457: [LSR] Use evaluateAtIteration in lsr-term-fold.

LGTM

All of the test changes are, to my eye, semantically the same.

I don't think they are: Let's say that %length == 0, which means that the BECount is -1. Previously, this just extended %length to i64, so that the effective trip count was zero. Now we instead extend -1 and then add the stride, making the trip count correct. The previous code was using an incorrect order of extensions.

So, you'd be correct here, except that the IV in question is nsw, and we branch on the result. For N=0, the starting value of the post-inc IV for the loop is -1 and we subtract 1 on every iteration. To reach zero, we have to pass through the negative overflow case, and thus have poison and (from the branch) UB. So, we can discount this case when reasoning about the expansion.

Tue, Mar 21, 7:40 AM · Restricted Project, Restricted Project
reames committed rG042783f55663: [LFTR] Assert and simplify under assumption exit counts are integers [nfc] (authored by reames).
[LFTR] Assert and simplify under assumption exit counts are integers [nfc]
Tue, Mar 21, 7:35 AM · Restricted Project, Restricted Project
reames closed D146470: [LFTR] Assert and simplify under assumption exit counts are integers.
Tue, Mar 21, 7:35 AM · Restricted Project, Restricted Project
reames added a comment to D134893: [LSR][TTI][RISCV] Add isAllowTerminatingConditionFoldingAfterLSR into TTI and enable it for RISC-V.

lsr-term-fold is still wildly unsound. I have patches out to address this, but we should wait until that is done discussing enabling it (by default or by target).

Tue, Mar 21, 7:13 AM · Restricted Project, Restricted Project

Mon, Mar 20

reames updated the diff for D146470: [LFTR] Assert and simplify under assumption exit counts are integers.

Another simplification.

Mon, Mar 20, 4:36 PM · Restricted Project, Restricted Project
reames updated the summary of D146470: [LFTR] Assert and simplify under assumption exit counts are integers.
Mon, Mar 20, 4:27 PM · Restricted Project, Restricted Project
reames requested review of D146470: [LFTR] Assert and simplify under assumption exit counts are integers.
Mon, Mar 20, 4:26 PM · Restricted Project, Restricted Project
reames requested review of D146468: [LFTR] Use evaluateAtIteration in genLoopLimit [nfc].
Mon, Mar 20, 4:15 PM · Restricted Project, Restricted Project
reames updated the summary of D146464: [LSR] Fix "new use of poison" problem in lsr-term-fold.
Mon, Mar 20, 3:16 PM · Restricted Project, Restricted Project
reames requested review of D146464: [LSR] Fix "new use of poison" problem in lsr-term-fold.
Mon, Mar 20, 3:10 PM · Restricted Project, Restricted Project
reames updated the summary of D146457: [LSR] Use evaluateAtIteration in lsr-term-fold.
Mon, Mar 20, 2:22 PM · Restricted Project, Restricted Project
reames requested review of D146457: [LSR] Use evaluateAtIteration in lsr-term-fold.
Mon, Mar 20, 2:17 PM · Restricted Project, Restricted Project
reames committed rG091422adc1d7: [LSR] Fix wrapping bug in lsr-term-fold logic (authored by reames).
[LSR] Fix wrapping bug in lsr-term-fold logic
Mon, Mar 20, 1:47 PM · Restricted Project, Restricted Project
reames closed D146429: [LSR] Fix wrapping bug in lsr-term-fold logic.
Mon, Mar 20, 1:47 PM · Restricted Project, Restricted Project
reames added inline comments to D146429: [LSR] Fix wrapping bug in lsr-term-fold logic.
Mon, Mar 20, 12:07 PM · Restricted Project, Restricted Project
reames added inline comments to D146429: [LSR] Fix wrapping bug in lsr-term-fold logic.
Mon, Mar 20, 11:24 AM · Restricted Project, Restricted Project
reames committed rG272ebd6957ef: [LSR] Inline getAlternateIVEnd and simplify [nfc] (authored by reames).
[LSR] Inline getAlternateIVEnd and simplify [nfc]
Mon, Mar 20, 11:23 AM · Restricted Project, Restricted Project
reames committed rGb9521484ec72: [LSR] Rewrite IV match for term-fold using existing utilities (authored by reames).
[LSR] Rewrite IV match for term-fold using existing utilities
Mon, Mar 20, 10:41 AM · Restricted Project, Restricted Project
reames committed rG67089a39a23b: [LSR] Regen tests to adjust for naming in SCEVExpander [nfc] (authored by reames).
[LSR] Regen tests to adjust for naming in SCEVExpander [nfc]
Mon, Mar 20, 9:39 AM · Restricted Project, Restricted Project
reames committed rG54539fa8b3a3: [LSR/LFTR] Move two utilities to common code for reuse [nfc] (authored by reames).
[LSR/LFTR] Move two utilities to common code for reuse [nfc]
Mon, Mar 20, 9:06 AM · Restricted Project, Restricted Project
reames requested review of D146429: [LSR] Fix wrapping bug in lsr-term-fold logic.
Mon, Mar 20, 8:12 AM · Restricted Project, Restricted Project

Fri, Mar 17

reames committed rG6f00170159f0: [LSR] Rework term-fold tests (authored by reames).
[LSR] Rework term-fold tests
Fri, Mar 17, 4:04 PM · Restricted Project, Restricted Project
reames accepted D146321: [RISCV] Add isReMaterializable to FLI instructions..

LGTM

Fri, Mar 17, 12:08 PM · Restricted Project, Restricted Project
reames added inline comments to D146321: [RISCV] Add isReMaterializable to FLI instructions..
Fri, Mar 17, 11:59 AM · Restricted Project, Restricted Project
reames accepted D146315: [RISCV] Add test case showing fli being hoisted out of a loop and creating extra copies/spills..

LGTM

Fri, Mar 17, 11:58 AM · Restricted Project, Restricted Project
reames accepted D146314: [RISCV] Add isAsCheapAsAMove to FLI instructions..

LGTM

Fri, Mar 17, 11:57 AM · Restricted Project, Restricted Project
reames committed rG3a1fb672f738: [LSR] Cleanup term-fold tests (authored by reames).
[LSR] Cleanup term-fold tests
Fri, Mar 17, 8:40 AM · Restricted Project, Restricted Project
reames committed rGc60e0b66ad24: [LSR] Another minor code style improvement [nfc] (authored by reames).
[LSR] Another minor code style improvement [nfc]
Fri, Mar 17, 8:09 AM · Restricted Project, Restricted Project
reames committed rGcd47f5bb5987: [LSR] Minor code style improvement [nfc] (authored by reames).
[LSR] Minor code style improvement [nfc]
Fri, Mar 17, 7:51 AM · Restricted Project, Restricted Project

Thu, Mar 16

reames committed rG502ab13eb036: [LSR] Add tests which demonstrate miscompiles in the current term-fold code (authored by reames).
[LSR] Add tests which demonstrate miscompiles in the current term-fold code
Thu, Mar 16, 2:10 PM · Restricted Project, Restricted Project