Page MenuHomePhabricator

david-arm (David Sherwood)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 20 2019, 6:41 AM (72 w, 5 d)

Recent Activity

Fri, Apr 9

david-arm updated the diff for D99727: [NFC][LoopVectorize] Remove unnecessary VF.isScalable asserts.
  • Removed extra assert for case where we are interleaving with masks
Fri, Apr 9, 8:21 AM · Restricted Project
david-arm updated the diff for D99951: [NFC] Add scalable vectorisation tests for int/FP <> int/FP conversions.
  • Removed warnings check line.
  • Combined all tests into one file.
Fri, Apr 9, 7:43 AM · Restricted Project
david-arm added a comment to D99951: [NFC] Add scalable vectorisation tests for int/FP <> int/FP conversions.

Sure, I can combine them into a single test if you think that's better? I wasn't sure whether in general we prefer to have more concise test files or to have fewer, larger files. I could create a file with a name like sve-type-conv.ll.

Fri, Apr 9, 6:09 AM · Restricted Project
david-arm added a reviewer for D99727: [NFC][LoopVectorize] Remove unnecessary VF.isScalable asserts: c-rhodes.
Fri, Apr 9, 6:02 AM · Restricted Project
david-arm added a comment to D99727: [NFC][LoopVectorize] Remove unnecessary VF.isScalable asserts.

Hi @frasercrmck yeah that's right, we can multiply an invalid cost by another value and the cost remains invalid.

Fri, Apr 9, 6:01 AM · Restricted Project
david-arm added a reviewer for D99935: [AArch64] Add instruction costs for FP_TO_UINT and FP_TO_SINT with half types: c-rhodes.
Fri, Apr 9, 5:58 AM · Restricted Project
david-arm added a reviewer for D99951: [NFC] Add scalable vectorisation tests for int/FP <> int/FP conversions: CarolineConcatto.
Fri, Apr 9, 5:58 AM · Restricted Project
david-arm added a comment to D100088: [DAGCombiner] Fold step_vector with add/mul/shl.

The folds here look sensible to me! I just have a minor comment about the add fold.

Fri, Apr 9, 5:57 AM · Restricted Project

Thu, Apr 8

david-arm added a comment to D94444: [RFC][Scalable] Add scalable shuffle intrinsic to extract evens from a pair of vectors.

Hi @cameron.mcinally I don't suppose you have any plans to do more work on this in the near future?

Thu, Apr 8, 7:23 AM · Restricted Project
david-arm committed rG1206313f82f8: [CodeGen][AArch64] Fix isel crash for truncating FP stores (authored by david-arm).
[CodeGen][AArch64] Fix isel crash for truncating FP stores
Thu, Apr 8, 5:22 AM
david-arm closed D100025: [CodeGen][AArch64] Fix isel crash for truncating FP stores.
Thu, Apr 8, 5:22 AM · Restricted Project

Wed, Apr 7

david-arm updated the diff for D100025: [CodeGen][AArch64] Fix isel crash for truncating FP stores.
  • Added a comment and removed warnings check from test.
Wed, Apr 7, 8:29 AM · Restricted Project
david-arm added inline comments to D100025: [CodeGen][AArch64] Fix isel crash for truncating FP stores.
Wed, Apr 7, 6:56 AM · Restricted Project
david-arm updated the diff for D99254: [SVE][LoopVectorize] Fix crash in InnerLoopVectorizer::widenPHIInstruction.
  • I realised that there was still a broken case where the PHIs are treated as uniform, which leads us to fall into the path where the PHI is scalar after vectorisation. We can support the uniform case for scalable vectors. I've fixed the code to work for scalable vectors and added a test.
Wed, Apr 7, 6:35 AM · Restricted Project
david-arm requested review of D100025: [CodeGen][AArch64] Fix isel crash for truncating FP stores.
Wed, Apr 7, 1:52 AM · Restricted Project
david-arm accepted D99682: [SelectionDAG] Teach SelectionDAG::FoldConstantArithmetic to handle SPLAT_VECTOR.

Ah, sorry, that was just a simple mistake and forget to "Accept Revision"!

Wed, Apr 7, 12:47 AM · Restricted Project

Tue, Apr 6

david-arm added a comment to D99682: [SelectionDAG] Teach SelectionDAG::FoldConstantArithmetic to handle SPLAT_VECTOR.

LGTM! Thanks a lot for making the changes.

Tue, Apr 6, 7:41 AM · Restricted Project
david-arm requested review of D99951: [NFC] Add scalable vectorisation tests for int/FP <> int/FP conversions.
Tue, Apr 6, 6:30 AM · Restricted Project
david-arm added a reviewer for D99935: [AArch64] Add instruction costs for FP_TO_UINT and FP_TO_SINT with half types: dmgreen.
Tue, Apr 6, 3:18 AM · Restricted Project
david-arm requested review of D99935: [AArch64] Add instruction costs for FP_TO_UINT and FP_TO_SINT with half types.
Tue, Apr 6, 3:18 AM · Restricted Project

Thu, Apr 1

david-arm accepted D99586: [AArch64] Default to zero-cycle-zeroing FP registers..

LGTM too!

Thu, Apr 1, 9:16 AM · Restricted Project
david-arm added inline comments to D99682: [SelectionDAG] Teach SelectionDAG::FoldConstantArithmetic to handle SPLAT_VECTOR.
Thu, Apr 1, 8:44 AM · Restricted Project
david-arm added inline comments to D99586: [AArch64] Default to zero-cycle-zeroing FP registers..
Thu, Apr 1, 8:32 AM · Restricted Project
david-arm requested review of D99727: [NFC][LoopVectorize] Remove unnecessary VF.isScalable asserts.
Thu, Apr 1, 7:02 AM · Restricted Project
david-arm added inline comments to D99569: [LoopVectorize] Fix bug where predicated loads/stores were dropped.
Thu, Apr 1, 6:52 AM · Restricted Project
david-arm added a comment to D99718: [LoopVectorize] Simplify scalar cost calculation in getInstructionCost.

A version of this patch was previously merged (D98512) then reverted due to a failure with the X86 sanitiser build that exposed some missing tests from our LLVM test suite regarding pointer PHIs feeding directly into stores. I've attempted another fix here without the previous assert because the logic seems far too complicated for an assert.

Thu, Apr 1, 4:17 AM · Restricted Project
david-arm requested review of D99718: [LoopVectorize] Simplify scalar cost calculation in getInstructionCost.
Thu, Apr 1, 4:15 AM · Restricted Project
david-arm added inline comments to D99699: [AArch64][SVE] Lowering sve.dot to DOT node.
Thu, Apr 1, 4:06 AM · Restricted Project
david-arm added inline comments to D99710: [AArch64] Use 64-bit movi for zeroing halfs/floats.
Thu, Apr 1, 2:54 AM · Restricted Project
david-arm committed rGe3a13304fc03: [NFC] Add tests for scalable vectorization of loops with large stride acesses (authored by david-arm).
[NFC] Add tests for scalable vectorization of loops with large stride acesses
Thu, Apr 1, 2:27 AM
david-arm closed D99192: [NFC] Add tests for scalable vectorization of loops with large stride acesses.
Thu, Apr 1, 2:26 AM · Restricted Project
david-arm added inline comments to D99682: [SelectionDAG] Teach SelectionDAG::FoldConstantArithmetic to handle SPLAT_VECTOR.
Thu, Apr 1, 1:46 AM · Restricted Project
david-arm added inline comments to D99192: [NFC] Add tests for scalable vectorization of loops with large stride acesses.
Thu, Apr 1, 1:11 AM · Restricted Project

Tue, Mar 30

david-arm added a reviewer for D99192: [NFC] Add tests for scalable vectorization of loops with large stride acesses: CarolineConcatto.
Tue, Mar 30, 3:20 AM · Restricted Project
david-arm committed rGa08c7736a771: [LoopVectorize] Add support for scalable vectorization of induction variables (authored by david-arm).
[LoopVectorize] Add support for scalable vectorization of induction variables
Tue, Mar 30, 3:14 AM
david-arm closed D98715: [LoopVectorize] Add support for scalable vectorization of induction variables.
Tue, Mar 30, 3:13 AM · Restricted Project

Fri, Mar 26

david-arm accepted D98934: [SVE][CostModel] Add instruction cost for operations on scalable vectors.

LGTM with nit addressed!

Fri, Mar 26, 9:24 AM · Restricted Project
david-arm accepted D98939: [SelectionDAG][AArch64][SVE] Perform SETCC condition legalization in LegalizeVectorOps.

LGTM! Thanks for the changes!

Fri, Mar 26, 9:20 AM · Restricted Project
david-arm added inline comments to D98509: [LV] Calculate max feasible scalable VF..
Fri, Mar 26, 9:15 AM · Restricted Project
david-arm added a reverting change for rG240aa96cf25d: [LoopVectorize] Simplify scalar cost calculation in getInstructionCost: rGc39460cc4f7c: Revert "[LoopVectorize] Simplify scalar cost calculation in getInstructionCost".
Fri, Mar 26, 4:37 AM
david-arm committed rGc39460cc4f7c: Revert "[LoopVectorize] Simplify scalar cost calculation in getInstructionCost" (authored by david-arm).
Revert "[LoopVectorize] Simplify scalar cost calculation in getInstructionCost"
Fri, Mar 26, 4:37 AM
david-arm added a reverting change for D98512: [LoopVectorize] Simplify scalar cost calculation in getInstructionCost: rGc39460cc4f7c: Revert "[LoopVectorize] Simplify scalar cost calculation in getInstructionCost".
Fri, Mar 26, 4:37 AM · Restricted Project
david-arm committed rG240aa96cf25d: [LoopVectorize] Simplify scalar cost calculation in getInstructionCost (authored by david-arm).
[LoopVectorize] Simplify scalar cost calculation in getInstructionCost
Fri, Mar 26, 4:27 AM
david-arm closed D98512: [LoopVectorize] Simplify scalar cost calculation in getInstructionCost.
Fri, Mar 26, 4:27 AM · Restricted Project
david-arm added a comment to D98934: [SVE][CostModel] Add instruction cost for operations on scalable vectors.

Hi @nasherm, thanks for all the good fixes here! I think it looks very close now - just have a couple of minor comments. I'm fine with most of the costs as the most important thing for now is to avoid crashing and we can refine these later if necessary. However, I think the truncation costs probably do need fixing.

Fri, Mar 26, 4:11 AM · Restricted Project
david-arm added inline comments to D99384: [AArch64] Avoid SCALAR_TO_VECTOR for single FP constant vector..
Fri, Mar 26, 2:32 AM · Restricted Project
david-arm added a comment to D98963: [LoopVectorize] Change the identity element for FAdd.

Ah sorry, I realise now you meant change PHIs (vector or scalar) with nsz and -0.0 to use +0.0 instead!

Fri, Mar 26, 1:54 AM · Restricted Project
david-arm added a comment to D98963: [LoopVectorize] Change the identity element for FAdd.

Hi @dmgreen and @spatel, I've been trying to follow the discussion but I'm not entirely sure I follow what you're proposing @kmclaughlin should do here? Are you suggesting that Kerry could change this patch to:

Fri, Mar 26, 1:49 AM · Restricted Project
david-arm accepted D98435: [LoopVectorize] Add strict in-order reduction support for fixed-width vectorization.

LGTM as well! Can address Dave's comment before merging I think?

Fri, Mar 26, 1:34 AM · Restricted Project

Wed, Mar 24

david-arm updated the diff for D98512: [LoopVectorize] Simplify scalar cost calculation in getInstructionCost.
Wed, Mar 24, 8:16 AM · Restricted Project
david-arm added a comment to D98506: [SVE][LoopVectorize] Verify support for vectorizing loops with invariant loads.

LGTM!

Wed, Mar 24, 6:45 AM · Restricted Project
david-arm added inline comments to D98939: [SelectionDAG][AArch64][SVE] Perform SETCC condition legalization in LegalizeVectorOps.
Wed, Mar 24, 6:40 AM · Restricted Project
david-arm requested review of D99254: [SVE][LoopVectorize] Fix crash in InnerLoopVectorizer::widenPHIInstruction.
Wed, Mar 24, 4:25 AM · Restricted Project
david-arm added inline comments to D98435: [LoopVectorize] Add strict in-order reduction support for fixed-width vectorization.
Wed, Mar 24, 2:32 AM · Restricted Project

Tue, Mar 23

david-arm added a reviewer for D98715: [LoopVectorize] Add support for scalable vectorization of induction variables: peterwaller-arm.
Tue, Mar 23, 9:50 AM · Restricted Project
david-arm added a reviewer for D99192: [NFC] Add tests for scalable vectorization of loops with large stride acesses: fhahn.
Tue, Mar 23, 8:27 AM · Restricted Project
david-arm requested review of D99192: [NFC] Add tests for scalable vectorization of loops with large stride acesses.
Tue, Mar 23, 8:26 AM · Restricted Project
david-arm added inline comments to D99074: [llvm][AArch64][SVE] Fold literals into math instructions.
Tue, Mar 23, 7:02 AM · Restricted Project
david-arm committed rGd70251163f71: [LoopVectorize][NFC] Refactor code to use IRBuilder::CreateStepVector (authored by david-arm).
[LoopVectorize][NFC] Refactor code to use IRBuilder::CreateStepVector
Tue, Mar 23, 4:29 AM
david-arm closed D97861: [LoopVectorize][NFC] Refactor code to use IRBuilder::CreateStepVector.
Tue, Mar 23, 4:29 AM · Restricted Project
david-arm closed D97299: [IR][SVE] Add new llvm.experimental.stepvector intrinsic.

Closed by commit 748ae5281d4f7f0ff261ba9e8c57e6b6fcfdb31e

Tue, Mar 23, 3:47 AM · Restricted Project
david-arm committed rG748ae5281d4f: [IR][SVE] Add new llvm.experimental.stepvector intrinsic (authored by david-arm).
[IR][SVE] Add new llvm.experimental.stepvector intrinsic
Tue, Mar 23, 3:43 AM
david-arm added a comment to D98934: [SVE][CostModel] Add instruction cost for operations on scalable vectors.

Hi @nasherm, thanks for addressing the previous review comments - it looks much better now! I just have one more comment about the estimated costs in the table.

Tue, Mar 23, 2:46 AM · Restricted Project

Mon, Mar 22

david-arm updated the diff for D97299: [IR][SVE] Add new llvm.experimental.stepvector intrinsic.
  • Added verifier tests.
Mon, Mar 22, 7:48 AM · Restricted Project
david-arm accepted D98351: [InstCombine] shrinkFPConstantVector(): scalable vector support.

LGTM! Hi @nasherm, thanks for addressing all the review comments! I just have a few minor nits.

Mon, Mar 22, 7:20 AM · Restricted Project
david-arm added a comment to D97299: [IR][SVE] Add new llvm.experimental.stepvector intrinsic.
  • Updated LangRef to better describe the intrinsic behaviour.

Thanks. I think it should be possible to enforce the new restrictions in the IR verifier, to prevent users from accidentally using them with wrong types.

Mon, Mar 22, 6:55 AM · Restricted Project
david-arm updated the summary of D98512: [LoopVectorize] Simplify scalar cost calculation in getInstructionCost.
Mon, Mar 22, 5:31 AM · Restricted Project
david-arm updated the summary of D98512: [LoopVectorize] Simplify scalar cost calculation in getInstructionCost.
Mon, Mar 22, 5:30 AM · Restricted Project
david-arm updated the diff for D98512: [LoopVectorize] Simplify scalar cost calculation in getInstructionCost.
  • Added an assert that if an instruction remains scalar after vectorisation, then both the instruction and all it's users will not be scalarised. If any users were to be scalarised then potentially that means multiple copies of the instruction will be generated. In the case of GEPs the cost is actually calculated as part of the load/store cost.
Mon, Mar 22, 5:29 AM · Restricted Project
david-arm added a comment to D98963: [LoopVectorize] Change the identity element for FAdd.

Why not just always use -0.0?

Mon, Mar 22, 5:10 AM · Restricted Project
david-arm updated the diff for D97299: [IR][SVE] Add new llvm.experimental.stepvector intrinsic.
  • Updated LangRef to better describe the intrinsic behaviour.
Mon, Mar 22, 5:10 AM · Restricted Project
david-arm added inline comments to D98934: [SVE][CostModel] Add instruction cost for operations on scalable vectors.
Mon, Mar 22, 2:28 AM · Restricted Project

Fri, Mar 19

david-arm added inline comments to D98715: [LoopVectorize] Add support for scalable vectorization of induction variables.
Fri, Mar 19, 6:53 AM · Restricted Project
david-arm updated the diff for D98715: [LoopVectorize] Add support for scalable vectorization of induction variables.
  • Fixed buildScalarSteps so that generate the full vector part for VF=(1,scalable).
Fri, Mar 19, 6:52 AM · Restricted Project
david-arm added inline comments to D98934: [SVE][CostModel] Add instruction cost for operations on scalable vectors.
Fri, Mar 19, 4:16 AM · Restricted Project
david-arm added inline comments to D98934: [SVE][CostModel] Add instruction cost for operations on scalable vectors.
Fri, Mar 19, 2:48 AM · Restricted Project
david-arm added inline comments to D98781: [AArch64] Enable UseAA globally in the AArch64 backend.
Fri, Mar 19, 2:23 AM · Restricted Project

Thu, Mar 18

david-arm updated the diff for D97299: [IR][SVE] Add new llvm.experimental.stepvector intrinsic.
  • Changed code to check for element sizes >= 8 bits instead of 1!
Thu, Mar 18, 10:23 AM · Restricted Project
david-arm accepted D98690: [AArch64][SVE] Test more types in sve-fixed-length-subvector.ll.

LGTM!

Thu, Mar 18, 3:52 AM · Restricted Project

Wed, Mar 17

david-arm added a comment to D98708: [LoopVectorize] relax FMF constraint for FP induction.

Yeah I'm fine with that if @dmgreen is happy? It makes to be consistent with the RecurrenceDescriptor code. I think from what I understand clang will only generate IR that contains the reassoc flag if we've set all the appropriate frontend flags. Therefore, currently the only ambiguity at the moment is when hand-writing IR and using the reassoc flag without nsz, right?

Wed, Mar 17, 5:47 AM · Restricted Project
david-arm added inline comments to D98351: [InstCombine] shrinkFPConstantVector(): scalable vector support.
Wed, Mar 17, 3:50 AM · Restricted Project
david-arm added a comment to D98708: [LoopVectorize] relax FMF constraint for FP induction.

Hi @dmgreen, yes of course you're right. I'd forgotten about the nsz requirement. It's definitely needed at compile time for vectorising FP reduction loops, i.e. clang -freassociative-math -fno-trapping-math -fno-signed-zeroes. I guess adding a check for nsz here is consistent with that?

Wed, Mar 17, 3:38 AM · Restricted Project
david-arm updated the diff for D97299: [IR][SVE] Add new llvm.experimental.stepvector intrinsic.
  • Addressed review comments.
  • After recent upstream discussion on the SVE sync call it was decided to remove support for vectors of i1 types.
Wed, Mar 17, 2:57 AM · Restricted Project
david-arm added inline comments to D98690: [AArch64][SVE] Test more types in sve-fixed-length-subvector.ll.
Wed, Mar 17, 2:47 AM · Restricted Project
david-arm accepted D98708: [LoopVectorize] relax FMF constraint for FP induction.

LGTM!

Wed, Mar 17, 2:28 AM · Restricted Project

Tue, Mar 16

david-arm added inline comments to D98715: [LoopVectorize] Add support for scalable vectorization of induction variables.
Tue, Mar 16, 9:27 AM · Restricted Project
david-arm added inline comments to D98435: [LoopVectorize] Add strict in-order reduction support for fixed-width vectorization.
Tue, Mar 16, 9:06 AM · Restricted Project
david-arm requested review of D98715: [LoopVectorize] Add support for scalable vectorization of induction variables.
Tue, Mar 16, 8:45 AM · Restricted Project
david-arm added inline comments to D98435: [LoopVectorize] Add strict in-order reduction support for fixed-width vectorization.
Tue, Mar 16, 5:02 AM · Restricted Project

Mon, Mar 15

david-arm added a comment to D98506: [SVE][LoopVectorize] Verify support for vectorizing loops with invariant loads.

Hi @CarolineConcatto, it sounds like a good idea to the reverse vector case, but I guess that probably depends upon your patch (D95363)?

Mon, Mar 15, 9:25 AM · Restricted Project
david-arm added inline comments to D98495: [CodeGen] Fix issues with scalable-vector INSERT/EXTRACT_SUBVECTORs.
Mon, Mar 15, 8:22 AM · Restricted Project
david-arm added inline comments to D98495: [CodeGen] Fix issues with scalable-vector INSERT/EXTRACT_SUBVECTORs.
Mon, Mar 15, 8:11 AM · Restricted Project
david-arm added a comment to D98512: [LoopVectorize] Simplify scalar cost calculation in getInstructionCost.

It looks like the only change is the IV update which goes from 4 -> 1. I think the adds you mentioned are included as part of the load/store cost.

I am not sure. From a quick look at the code, it seems like the high load/store cost comes mostly from the required scalarization of the stored/loaded values and the cost of the extra increments seems not fully included at least on the IR instruction level. Unfortunately I don't think there's really any different option to write a test that hits the code paths in the patch, even if the scalarized values are used in other places later on. So it's probably not worth worrying too much about it. I still think we should at least have an assertion as mentioned above, to catch the gap if the cost-modeling is improved.

Mon, Mar 15, 6:17 AM · Restricted Project
david-arm abandoned D91957: [Support] Migrate more high level cost functions to using InstructionCost.
Mon, Mar 15, 5:53 AM · Restricted Project
david-arm added a comment to D98512: [LoopVectorize] Simplify scalar cost calculation in getInstructionCost.

Hi @fhahn, I tested the example you pasted above before and after applying my patch and got this debug output:

Mon, Mar 15, 3:54 AM · Restricted Project
david-arm added a comment to D98512: [LoopVectorize] Simplify scalar cost calculation in getInstructionCost.

Although I think the adds you mentioned above @fhahn don't fall into the Scalars variable - I'll double check this though. This patch only affects instructions that are members of Scalars. Users of the IV don't fall into that - only updates to the IV I think.

Mon, Mar 15, 3:36 AM · Restricted Project
david-arm added a comment to D98512: [LoopVectorize] Simplify scalar cost calculation in getInstructionCost.

Thanks for the review @fhahn. If anything this just highlights the poor code coverage we have at the moment for cost calculations when scalarising. At the very least I'll add some tests that cover some of these corner cases, since there was nothing that really broke when I made this change!

Mon, Mar 15, 3:34 AM · Restricted Project
david-arm added a comment to D98512: [LoopVectorize] Simplify scalar cost calculation in getInstructionCost.

I guess for bitcasts the cost may be non-trivial for big endian targets.

Mon, Mar 15, 2:55 AM · Restricted Project
david-arm added a comment to D98512: [LoopVectorize] Simplify scalar cost calculation in getInstructionCost.

Hi @dmgreen, thanks for taking a look at the patch. So you raise a good point - the loads/stores will be dealt with before reaching this variant getInstructionCost, however the GEPs/Bitcasts for those will still be costed here. However, for GEPs at least you can see this code in getInstructionCost:

Mon, Mar 15, 2:46 AM · Restricted Project

Mar 12 2021

david-arm added a comment to D98054: [LoopVectorize][SVE] Fix crash when vectorising FP negation.

Hi @sdesmalen, thanks for pointing this out! It looks like for the cases you listed above we actually have:

Mar 12 2021, 8:29 AM · Restricted Project