Page MenuHomePhabricator

samparker (Sam Parker)
part-timer

Projects

User does not belong to any projects.

User Details

User Since
May 11 2015, 7:59 AM (296 w, 5 d)

Recent Activity

Yesterday

samparker added inline comments to D93694: [MachineLICM][MachineSink] Move SinkIntoLoop from MachineLICM to MachineSink.
Fri, Jan 15, 6:51 AM · Restricted Project
samparker added reviewers for D93694: [MachineLICM][MachineSink] Move SinkIntoLoop from MachineLICM to MachineSink: shchenz, qcolombet.
Fri, Jan 15, 6:03 AM · Restricted Project
samparker added a comment to D92934: [ARM][MachineOutliner] Add stack fixup feature..

Sorry for the big delay Yvan, I haven't really been working on LLVM recently.

Fri, Jan 15, 1:33 AM · Restricted Project

Dec 17 2020

samparker accepted D92933: [ARM][MachineOutliner] Fix costs model..
Dec 17 2020, 5:25 AM · Restricted Project
samparker added inline comments to D92933: [ARM][MachineOutliner] Fix costs model..
Dec 17 2020, 5:04 AM · Restricted Project
samparker added inline comments to D92933: [ARM][MachineOutliner] Fix costs model..
Dec 17 2020, 3:13 AM · Restricted Project

Dec 11 2020

samparker added inline comments to D92933: [ARM][MachineOutliner] Fix costs model..
Dec 11 2020, 1:18 AM · Restricted Project

Dec 10 2020

samparker added inline comments to D92933: [ARM][MachineOutliner] Fix costs model..
Dec 10 2020, 12:35 AM · Restricted Project

Dec 8 2020

samparker added a comment to D92094: [CostModel]Replace FixedVectorType by VectorType in costgetIntrinsicInstrCost.

Please move the if (isa<ScalableVectorType>(RetTy)) return BaseT::getIntrinsicInstrCost(ICA, CostKind); bailing out logic into the beginning of each switch branch that does not work/is not currently tested. These can be removed on a case-by-case basis in the patches that fix these branches.

+1

Dec 8 2020, 12:52 AM · Restricted Project

Dec 3 2020

samparker added inline comments to D92385: [ARM] Add a pass that re-arranges blocks when there is a backwards WLS branch.
Dec 3 2020, 2:49 AM · Restricted Project

Dec 2 2020

samparker added inline comments to D92385: [ARM] Add a pass that re-arranges blocks when there is a backwards WLS branch.
Dec 2 2020, 7:18 AM · Restricted Project
samparker added a comment to D90836: [ARM][MachineOutliner] Add stack fixup feature..

Hi Yvan,

Dec 2 2020, 7:01 AM · Restricted Project
samparker added inline comments to D92385: [ARM] Add a pass that re-arranges blocks when there is a backwards WLS branch.
Dec 2 2020, 5:21 AM · Restricted Project
samparker added inline comments to D92385: [ARM] Add a pass that re-arranges blocks when there is a backwards WLS branch.
Dec 2 2020, 12:58 AM · Restricted Project
samparker added inline comments to D92385: [ARM] Add a pass that re-arranges blocks when there is a backwards WLS branch.
Dec 2 2020, 12:57 AM · Restricted Project

Dec 1 2020

samparker added inline comments to D92385: [ARM] Add a pass that re-arranges blocks when there is a backwards WLS branch.
Dec 1 2020, 5:53 AM · Restricted Project
samparker accepted D92159: [LSR][NFC] don't collect chains when isNumRegsMajorCostOfLSR is false.

Okay, fair enough.

Dec 1 2020, 1:09 AM · Restricted Project

Nov 30 2020

samparker added a comment to D92094: [CostModel]Replace FixedVectorType by VectorType in costgetIntrinsicInstrCost.

I'd prefer if we didn't knowingly create broken paths here... does it currently just crash in the AArch64 backend or are there also failures in the generic parts? I'm thinking that we could just have a separate switch statement which checks for supported opcodes for scalable vectors and protects us from going down bad paths. A TODO comment could also be added to document why and what needs fixing up.

Nov 30 2020, 2:45 AM · Restricted Project

Nov 26 2020

samparker added a comment to D92094: [CostModel]Replace FixedVectorType by VectorType in costgetIntrinsicInstrCost.

I think it would be a good idea to test that the paths through getCmpSelInstrCost and getArithmeticInstrCost work too.

Nov 26 2020, 1:40 AM · Restricted Project

Nov 24 2020

samparker accepted D91724: [HardwareLoops] Change order of SCEV expression construction for InitLoopCount..

Intuitively, it sounds unlikely for getExitCount to return a pointer type but I'm not sure.

Indeed, thanks for checking. LGTM, thanks.

Nov 24 2020, 1:01 AM · Restricted Project

Nov 23 2020

samparker added a comment to D91481: [LoopUnroll] Discount uniform instructions in cost models.

This looks sensible to me and a step in the right direction. Have you ran any benchmarks? Would be good to know, at least, that this isn't causing any obvious regressions.

Nov 23 2020, 1:27 AM · Restricted Project

Nov 20 2020

samparker added a comment to D89800: [ARM][LowOverheadLoops] Don't generate a LOL if lr is redefined after the start.

I'd be happy with an assertion that DoLoopStart is connected properly, but WhileLoopStart is still my real concern with this logic.

Nov 20 2020, 7:21 AM · Restricted Project
samparker added inline comments to D91857: [ARM] Remove dead mov's in preheader of tail predicated loops.
Nov 20 2020, 7:06 AM · Restricted Project
samparker added a comment to D89800: [ARM][LowOverheadLoops] Don't generate a LOL if lr is redefined after the start.

So my thoughts:

  • For the case where LRDef is not null, we should be able to check that this same value is used by 'Dec'. That way we have proved explicitly the the use-def chain is correct instead of relying on the check of the live-out value.
  • For any the case where LRDef is null, I'm pretty sure we just need to try harder to find it - something needs to be setting LR for loop entry.
  • My issue with the isSafeToDefRegAt is that is a local check. If WLS is in the preheader predecessor, we could decide we that defining LR is safe (locally), but then LR is redefined in the preheader.
Nov 20 2020, 6:26 AM · Restricted Project

Nov 19 2020

samparker added inline comments to D91724: [HardwareLoops] Change order of SCEV expression construction for InitLoopCount..
Nov 19 2020, 7:01 AM · Restricted Project
samparker added a comment to D91724: [HardwareLoops] Change order of SCEV expression construction for InitLoopCount..

I wonder whether the zero-extend should also be moved to isHardwareLoopCandidate at this point.

Nice, sounds good to me.

Nov 19 2020, 6:50 AM · Restricted Project
samparker added a comment to D91724: [HardwareLoops] Change order of SCEV expression construction for InitLoopCount..

Out of curiosity, did you try performing the +1 in HardwareLoopInfo::isHardwareLoopCandidate instead? Looking back through the code, I think it would be more clear to rename 'ExitCount' to 'TripCount' and perform this addition outside of the pass.

Nov 19 2020, 12:58 AM · Restricted Project
samparker accepted D91724: [HardwareLoops] Change order of SCEV expression construction for InitLoopCount..

LGTM, thanks!

Nov 19 2020, 12:55 AM · Restricted Project

Nov 18 2020

samparker accepted D90935: [ARM][LowOverheadLoops] Merge VCMP and VPST across VPT blocks.

I'm happy if Dave has no other suggestions.

Nov 18 2020, 2:47 AM · Restricted Project
samparker accepted D91663: [ARM] Disable WLSTP loops.

Sounds good to me, I would have been in favour of just completely disabling wls for now :)

Nov 18 2020, 12:42 AM · Restricted Project

Nov 17 2020

samparker added inline comments to D90935: [ARM][LowOverheadLoops] Merge VCMP and VPST across VPT blocks.
Nov 17 2020, 8:52 AM · Restricted Project
samparker added inline comments to D90935: [ARM][LowOverheadLoops] Merge VCMP and VPST across VPT blocks.
Nov 17 2020, 2:35 AM · Restricted Project

Nov 16 2020

samparker added a comment to D91450: [BPI] Look through bitcasts in calcZeroHeuristic.

But now I understand that this isn't an NFC! So can you add a test please?

Nov 16 2020, 4:13 AM · Restricted Project
samparker accepted D91450: [BPI] Look through bitcasts in calcZeroHeuristic.

Thanks. It's okay to commit cleanup NFCs without review, so don't worry about these kind of patches up for review in the future.

Nov 16 2020, 4:11 AM · Restricted Project

Nov 12 2020

samparker accepted D91287: [ARM] Ensure CountReg definition dominates InsertPt when creating t2DoLoopStartTP.

Ok, cheers! LGTM

Nov 12 2020, 3:51 AM · Restricted Project
samparker added inline comments to D91287: [ARM] Ensure CountReg definition dominates InsertPt when creating t2DoLoopStartTP.
Nov 12 2020, 1:59 AM · Restricted Project

Nov 11 2020

samparker added a comment to D91267: [ARM] Remove copies from low overhead phi inductions..

Getting phis to use the right register class sounds like a good generic change to make. If creating them like that is a pain, how about just moving most of this logic into OptimizePhis?

Nov 11 2020, 8:02 AM · Restricted Project
samparker accepted D91257: [ARM] Deliberately prevent inline asm in low overhead loops. NFC.

Explicit sounds good.

Nov 11 2020, 6:53 AM · Restricted Project
samparker committed rG898a81dfc544: [NFC][ARM] Replace lambda with any_of (authored by samparker).
[NFC][ARM] Replace lambda with any_of
Nov 11 2020, 2:03 AM

Nov 10 2020

samparker added a comment to D90554: [CostModel] remove cost-kind predicate for intrinsics in basic TTI implementation.

Yep, LGTM

Nov 10 2020, 12:37 AM · Restricted Project

Nov 9 2020

samparker accepted D90591: [ARM] Introduce t2DoLoopStartTP.
Nov 9 2020, 7:46 AM · Restricted Project
samparker added inline comments to D90591: [ARM] Introduce t2DoLoopStartTP.
Nov 9 2020, 4:03 AM · Restricted Project

Nov 6 2020

samparker added a comment to D90591: [ARM] Introduce t2DoLoopStartTP.

I think that the code would still need a t2DoLoopStartTP instruction, so those change would all have to stay

Agreed, and I think this pseudo is very good idea.

Nov 6 2020, 4:33 AM · Restricted Project

Nov 5 2020

samparker added a comment to D90781: [ARM] remove cost-kind predicate for cmp/sel costs.

and the MVE costs should be our problem not yours :)

Indeed! Thanks both for the explanations.

Nov 5 2020, 8:49 AM · Restricted Project
samparker added a comment to D90591: [ARM] Introduce t2DoLoopStartTP.

That's all about different patches though! Any comments on this one?

Sorry, it's just difficult to gauge this without a view on the wider implementation idea. For the other stuff that you're talking about, of course we need that in MI but, like I said at the start, we could currently achieve this patch with probably 10x less code at the IR level.

Nov 5 2020, 4:37 AM · Restricted Project
samparker added a comment to D90591: [ARM] Introduce t2DoLoopStartTP.

With the MVE instructions taking LR, maybe you wouldn't need to make the terminator change, have you tried it yet? I guess I'm naively assuming that the register allocator will be much less likely to spill something that is used by most / all of the MVE instructions within the loop.

Nov 5 2020, 3:08 AM · Restricted Project
samparker added inline comments to D90781: [ARM] remove cost-kind predicate for cmp/sel costs.
Nov 5 2020, 12:38 AM · Restricted Project

Nov 4 2020

samparker accepted D90606: [llvm][AArch64] Allow TB(N)Z to drop signext for sign bit tests..

Thanks, LGTM

Nov 4 2020, 12:29 AM · Restricted Project
samparker added a comment to D90591: [ARM] Introduce t2DoLoopStartTP.

I would like to put here start with combining t2LoopDec and t2LoopEnd

Ok, fair enough.

Nov 4 2020, 12:20 AM · Restricted Project

Nov 3 2020

samparker accepted D90681: [CostModel] fix cost calc bug for sadd/ssub with overflow.

Okay, fair enough! I was getting confused that this is still throughput only.

Nov 3 2020, 6:51 AM · Restricted Project
samparker added a comment to D90681: [CostModel] fix cost calc bug for sadd/ssub with overflow.

Thanks for doing this. But now I'm surprised to not see any Arm changes?

Nov 3 2020, 6:23 AM · Restricted Project
samparker added a comment to D90591: [ARM] Introduce t2DoLoopStartTP.

Things like reverting calls is easier to do at this point.

What calls do you mean..?
What specifically do you need from the pre-RA MachineInstr form that means this shouldn't be done at IR level? The search here is certainly not more simple and manageable than doing this MVETailPredication, where the components are also found, and handling phis at this level is just an added pain.

Nov 3 2020, 2:06 AM · Restricted Project
samparker added a comment to D90591: [ARM] Introduce t2DoLoopStartTP.

This sounds like a good idea to me, but if we're going to do stuff earlier, to make our lives easier, maybe we should do this at the IR level and use a target-specific intrinsic and free ourselves completely from searching at the MI level?

Nov 3 2020, 1:27 AM · Restricted Project
samparker added a comment to D90554: [CostModel] remove cost-kind predicate for intrinsics in basic TTI implementation.
Cost += 2 * thisT()->getCmpSelInstrCost(
                BinaryOperator::ICmp, OverflowTy, OverflowTy,
                CmpInst::BAD_ICMP_PREDICATE, CostKind);

It looks like this second ICmp should be a select/

Nov 3 2020, 1:09 AM · Restricted Project

Nov 2 2020

samparker added a comment to D90554: [CostModel] remove cost-kind predicate for intrinsics in basic TTI implementation.

Looks like the Arm backend makes almost no attempt to cost intrinsics, so I'm assuming that BasicTTI is basically producing everything here... So, it looks like there's some scalarization cost missing in this nebulous code?

Nov 2 2020, 2:12 AM · Restricted Project

Oct 28 2020

samparker added inline comments to D90162: [llvm][AArch64] Prevent spurious zero extension..
Oct 28 2020, 6:50 AM · Restricted Project

Oct 27 2020

samparker added inline comments to D90162: [llvm][AArch64] Prevent spurious zero extension..
Oct 27 2020, 7:44 AM · Restricted Project

Oct 26 2020

samparker accepted D89969: [SLP] Consider alternatives for cost of select instructions..

Probably best to wait a little while for other's feedback, but this LGTM.

Oct 26 2020, 2:03 AM · Restricted Project

Oct 23 2020

samparker accepted D89883: [ARM] Add a RegAllocHint for hinting t2DoLoopStart towards LR.

I'm kind of surprised these sort of hints aren't handled in tablegen, but I guess not much c++ is needed. Looks like it works well enough too, cheers.

Oct 23 2020, 12:33 AM · Restricted Project
samparker added inline comments to D89969: [SLP] Consider alternatives for cost of select instructions..
Oct 23 2020, 12:15 AM · Restricted Project

Oct 22 2020

samparker accepted D89881: [ARM] Alter t2DoLoopStart to define lr.

Many thanks Dave, LGTM

Oct 22 2020, 4:03 AM · Restricted Project
samparker added inline comments to D89800: [ARM][LowOverheadLoops] Don't generate a LOL if lr is redefined after the start.
Oct 22 2020, 3:57 AM · Restricted Project

Oct 21 2020

samparker added a comment to D89665: [LSR] ignore profitable chain optimization when instruction number is the major cost.

Thanks!

Oct 21 2020, 11:39 PM · Restricted Project
samparker requested review of D89894: [AArch64] Backedge indexing.
Oct 21 2020, 9:19 AM · Restricted Project
samparker accepted D89665: [LSR] ignore profitable chain optimization when instruction number is the major cost.

Having separate loops sounds good to me, but please remove the extra conditional before committing. Thanks.

Oct 21 2020, 8:05 AM · Restricted Project
samparker added inline comments to D89665: [LSR] ignore profitable chain optimization when instruction number is the major cost.
Oct 21 2020, 7:35 AM · Restricted Project
samparker added a comment to D89881: [ARM] Alter t2DoLoopStart to define lr.

A heroic effort to change all those tests! Are you planning on also updating WLS and the test_set intrinsic too..?

Oct 21 2020, 7:28 AM · Restricted Project
samparker added a comment to D89800: [ARM][LowOverheadLoops] Don't generate a LOL if lr is redefined after the start.

$lr = t2DoLoopStart renamable $rn

+1

Oct 21 2020, 12:59 AM · Restricted Project
samparker added inline comments to D89665: [LSR] ignore profitable chain optimization when instruction number is the major cost.
Oct 21 2020, 12:58 AM · Restricted Project

Oct 19 2020

samparker added a comment to D89693: [AArch64] Favor post-increments.

How about results from the LLVM test suite? This should benefit size and performance?

Oct 19 2020, 7:14 AM · Restricted Project
samparker committed rG03f3ef221b02: [LangRef] Correct return type llvm.test.set.loop.iterations.* (authored by samparker).
[LangRef] Correct return type llvm.test.set.loop.iterations.*
Oct 19 2020, 4:57 AM
samparker closed D89564: Provide correct return type and additional description for llvm.test.set.loop.iterations.* in langref.
Oct 19 2020, 4:57 AM · Restricted Project
samparker added a comment to D89564: Provide correct return type and additional description for llvm.test.set.loop.iterations.* in langref.

Yeah, I'm happy to do that for you.

Oct 19 2020, 4:18 AM · Restricted Project
samparker accepted D89564: Provide correct return type and additional description for llvm.test.set.loop.iterations.* in langref.

Thanks!

Oct 19 2020, 3:00 AM · Restricted Project
samparker added inline comments to D89549: [ARM][LowOverheadLoops] Check live-out for InsertPt instead of Start.
Oct 19 2020, 2:50 AM · Restricted Project

Oct 15 2020

samparker accepted D89461: [CostModel] remove cost-kind predicate for ctlz/cttz intrinsics in basic TTI implementation.

I'm sorry for not finishing what I started with this intrinsic nonsense... Any simplification of these winding paths sounds great to me.

Oct 15 2020, 7:00 AM · Restricted Project
samparker added a comment to D89048: [ARM][LowOverheadLoops] Insert loop start at end of block in more cases.

Sorry, I didn't realise this had been updated.

Oct 15 2020, 4:54 AM · Restricted Project

Oct 9 2020

samparker added inline comments to D89048: [ARM][LowOverheadLoops] Insert loop start at end of block in more cases.
Oct 9 2020, 2:01 AM · Restricted Project
samparker added inline comments to D89048: [ARM][LowOverheadLoops] Insert loop start at end of block in more cases.
Oct 9 2020, 1:59 AM · Restricted Project
samparker added a comment to D89048: [ARM][LowOverheadLoops] Insert loop start at end of block in more cases.

I wasn't so much concerned about latencies, but I was guessing that by moving the instruction, the goal is to get some more tail-predication "for free".

Indeed, this greatly simplifies the effort required to ensure that we can generate the LSTP version. And I can't see the latency of DLS having any real affect on the performance of a loop, especially not compared to all the other things that can go bad!

Oct 9 2020, 1:43 AM · Restricted Project

Oct 1 2020

samparker committed rG7e02bc81c6da: [NFC][ARM] LowOverheadLoop DEBUG statements (authored by samparker).
[NFC][ARM] LowOverheadLoop DEBUG statements
Oct 1 2020, 5:38 AM
samparker added a comment to D88494: Add "SkipDead" parameter to TargetInstrInfo::DefinesPredicate.

Can we rename the function to something like "ClobbersPredicate", if that's what we actually care about here?

+1 from me. I also wonder renaming IncludesRemovable to something like Ignore/SkipDead?

Oct 1 2020, 4:52 AM · Restricted Project
samparker accepted D42365: [LoopFlatten] Add a loop-flattening pass.

Maybe we could use SCEV for the overflow checks? ;P
Code looks good to me and we've been running this downstream for a few years.

Oct 1 2020, 3:04 AM · Restricted Project
samparker committed rG38f625d0d136: [ARM][LowOverheadLoops] Adjust Start insertion. (authored by samparker).
[ARM][LowOverheadLoops] Adjust Start insertion.
Oct 1 2020, 2:50 AM
samparker closed D88638: [ARM][LowOverheadLoops] Adjust Start insertion..
Oct 1 2020, 2:49 AM · Restricted Project
samparker committed rG6ec5f324973d: [ARM][LowOverheadLoops] Iteration count liveness (authored by samparker).
[ARM][LowOverheadLoops] Iteration count liveness
Oct 1 2020, 2:14 AM
samparker closed D88549: [ARM][LowOverheadLoops] Iteration count liveness.
Oct 1 2020, 2:14 AM · Restricted Project
samparker committed rG7b90516d479c: [ARM][LowOverheadLoops] Start insertion point (authored by samparker).
[ARM][LowOverheadLoops] Start insertion point
Oct 1 2020, 2:07 AM
samparker closed D88542: [ARM][LowOverheadLoops] Start insertion point.
Oct 1 2020, 2:07 AM · Restricted Project
samparker updated the diff for D88638: [ARM][LowOverheadLoops] Adjust Start insertion..

Removed the code that later tries to move stuff around.

Oct 1 2020, 1:28 AM · Restricted Project
samparker requested review of D88638: [ARM][LowOverheadLoops] Adjust Start insertion..
Oct 1 2020, 1:20 AM · Restricted Project
samparker added inline comments to D88542: [ARM][LowOverheadLoops] Start insertion point.
Oct 1 2020, 1:15 AM · Restricted Project
samparker updated the diff for D88542: [ARM][LowOverheadLoops] Start insertion point.

Rebased.

Oct 1 2020, 12:42 AM · Restricted Project
samparker committed rGdfa2c14b8fe8: [ARM][LowOverheadLoops] Use iterator for InsertPt. (authored by samparker).
[ARM][LowOverheadLoops] Use iterator for InsertPt.
Oct 1 2020, 12:33 AM

Sep 30 2020

samparker committed rG3f88c10a6b25: [RDA] isSafeToDefRegAt: Look at global uses (authored by samparker).
[RDA] isSafeToDefRegAt: Look at global uses
Sep 30 2020, 6:08 AM
samparker closed D88554: [RDA] isSafeToDefRegAt: Look at global uses.
Sep 30 2020, 6:08 AM · Restricted Project
samparker requested review of D88554: [RDA] isSafeToDefRegAt: Look at global uses.
Sep 30 2020, 4:28 AM · Restricted Project
samparker committed rG3cbd01ddb937: [NFC][ARM] Add more LowOverheadLoop tests. (authored by samparker).
[NFC][ARM] Add more LowOverheadLoop tests.
Sep 30 2020, 4:20 AM
samparker requested review of D88549: [ARM][LowOverheadLoops] Iteration count liveness.
Sep 30 2020, 3:15 AM · Restricted Project
samparker updated the diff for D88542: [ARM][LowOverheadLoops] Start insertion point.

Checking result of TryRemove.

Sep 30 2020, 2:43 AM · Restricted Project