Page MenuHomePhabricator

sdesmalen (Sander de Smalen)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 21 2016, 1:19 AM (221 w, 6 d)

Recent Activity

Yesterday

sdesmalen added inline comments to D94708: [IR] Introduce llvm.experimental.vector.splice intrinsic.
Wed, Jan 20, 7:15 AM · Restricted Project

Tue, Jan 19

sdesmalen accepted D94427: [NFC][InstructionCost] Use InstructionCost in lib/Transforms/IPO/IROutliner.cpp.

LGTM

Tue, Jan 19, 7:03 AM · Restricted Project
sdesmalen added inline comments to D94401: [AArch64][SVE] Allow accesses to SVE stack objects to use frame pointer.
Tue, Jan 19, 6:05 AM · Restricted Project
sdesmalen accepted D89239: [RISCV][PrologEpilogInserter] "Float" emergency spill slots to avoid making them immediately unreachable from the stack pointer.

LGTM, thanks @rogfer01!

Tue, Jan 19, 3:37 AM · Restricted Project
sdesmalen added inline comments to D92238: [SCEVExpander] Migrate costAndCollectOperands to use InstructionCost..
Tue, Jan 19, 1:20 AM · Restricted Project

Mon, Jan 18

sdesmalen added inline comments to D92238: [SCEVExpander] Migrate costAndCollectOperands to use InstructionCost..
Mon, Jan 18, 8:09 AM · Restricted Project
sdesmalen accepted D93639: [AArch64][SVE]Add cost model for vector reduce for scalable vector.

LGTM

Mon, Jan 18, 4:13 AM · Restricted Project
sdesmalen accepted D94065: [NFC] Make remaining cost functions in LoopVectorize.cpp use InstructionCost.

LGTM!

Mon, Jan 18, 3:09 AM · Restricted Project
sdesmalen added inline comments to D92238: [SCEVExpander] Migrate costAndCollectOperands to use InstructionCost..
Mon, Jan 18, 1:35 AM · Restricted Project
sdesmalen updated the diff for D92238: [SCEVExpander] Migrate costAndCollectOperands to use InstructionCost..
  • Reordered includes.
  • Changed two more variables to be of type InstructionCost instead of int.
Mon, Jan 18, 1:35 AM · Restricted Project
sdesmalen added inline comments to D93639: [AArch64][SVE]Add cost model for vector reduce for scalable vector.
Mon, Jan 18, 1:10 AM · Restricted Project

Sun, Jan 17

sdesmalen added inline comments to D88595: [TableGen] Add not_all_same constraint check.
Sun, Jan 17, 12:56 PM · Restricted Project
sdesmalen accepted D94142: [IR] Allow scalable vectors in structs to support intrinsics returning multiple values..

Thanks for the latest changes, the patch looks good to me now!

Sun, Jan 17, 12:53 PM · Restricted Project

Fri, Jan 15

sdesmalen requested review of D94776: [AArch64][SVE] Asm: Fix supported immediates for DUP/CPY.
Fri, Jan 15, 6:42 AM · Restricted Project

Thu, Jan 14

sdesmalen added a comment to D92238: [SCEVExpander] Migrate costAndCollectOperands to use InstructionCost..

Gentle ping

Thu, Jan 14, 8:51 AM · Restricted Project
sdesmalen added inline comments to D94142: [IR] Allow scalable vectors in structs to support intrinsics returning multiple values..
Thu, Jan 14, 6:38 AM · Restricted Project
sdesmalen accepted D94069: [NFC][InstructionCost]Migrate VectorCombine.cpp to use InstructionCost.

Added two nits, but LGTM otherwise.

Thu, Jan 14, 6:36 AM · Restricted Project
sdesmalen added a comment to D93639: [AArch64][SVE]Add cost model for vector reduce for scalable vector.

Thanks for the changes so far, I think the patch is nearly there now.

Thu, Jan 14, 5:05 AM · Restricted Project

Wed, Jan 13

sdesmalen added inline comments to D93639: [AArch64][SVE]Add cost model for vector reduce for scalable vector.
Wed, Jan 13, 9:52 AM · Restricted Project
sdesmalen added inline comments to D94444: [RFC][Scalable] Add scalable shuffle intrinsic to extract evens from a pair of vectors.
Wed, Jan 13, 9:26 AM · Restricted Project
sdesmalen updated subscribers of D94444: [RFC][Scalable] Add scalable shuffle intrinsic to extract evens from a pair of vectors.
Wed, Jan 13, 9:00 AM · Restricted Project
sdesmalen added inline comments to D94142: [IR] Allow scalable vectors in structs to support intrinsics returning multiple values..
Wed, Jan 13, 8:53 AM · Restricted Project
sdesmalen added inline comments to D94069: [NFC][InstructionCost]Migrate VectorCombine.cpp to use InstructionCost.
Wed, Jan 13, 7:32 AM · Restricted Project
sdesmalen added inline comments to D93639: [AArch64][SVE]Add cost model for vector reduce for scalable vector.
Wed, Jan 13, 7:10 AM · Restricted Project

Tue, Jan 12

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

I like that too. I also like David's suggestion about deinterleave.even. I don't really have a strong opinion on either though. Does anyone feel strongly either way?

I quite like Dave's suggestion for deinterleave.even/odd and interleave.lo/hi, because now they are related (antonyms) and the names are still intuitive.

Tue, Jan 12, 9:07 AM · Restricted Project
sdesmalen committed rGc8a914db5c60: [LiveDebugValues] Fix comparison operator in VarLocBasedImpl (authored by sdesmalen).
[LiveDebugValues] Fix comparison operator in VarLocBasedImpl
Tue, Jan 12, 12:46 AM
sdesmalen added a comment to D94444: [RFC][Scalable] Add scalable shuffle intrinsic to extract evens from a pair of vectors.

Thanks for creating this patch!

Tue, Jan 12, 12:08 AM · Restricted Project

Mon, Jan 11

sdesmalen added inline comments to D90046: [LiveDebugValues] Handle spill locations with a fixed and scalable component..
Mon, Jan 11, 1:22 PM · Restricted Project
sdesmalen accepted D92747: [DAGCombiner] Use getVectorElementCount inside visitINSERT_SUBVECTOR.

LGTM

Mon, Jan 11, 5:46 AM · Restricted Project

Fri, Jan 8

sdesmalen added inline comments to D94065: [NFC] Make remaining cost functions in LoopVectorize.cpp use InstructionCost.
Fri, Jan 8, 8:53 AM · Restricted Project
sdesmalen accepted D94301: [NFC] Remove min/max functions from InstructionCost.

LGTM

Fri, Jan 8, 8:15 AM · Restricted Project
sdesmalen accepted D94171: [SVE][CodeGen] Fix legalisation of floating-point masked gathers.
Fri, Jan 8, 8:13 AM · Restricted Project
sdesmalen accepted D92178: [NFC][InstructionCost] Change LoopVectorizationCostModel::getInstructionCost to return InstructionCost.
Fri, Jan 8, 6:21 AM · Restricted Project
sdesmalen added inline comments to D92747: [DAGCombiner] Use getVectorElementCount inside visitINSERT_SUBVECTOR.
Fri, Jan 8, 6:20 AM · Restricted Project
sdesmalen added inline comments to D94069: [NFC][InstructionCost]Migrate VectorCombine.cpp to use InstructionCost.
Fri, Jan 8, 6:11 AM · Restricted Project
sdesmalen added inline comments to D94142: [IR] Allow scalable vectors in structs to support intrinsics returning multiple values..
Fri, Jan 8, 5:16 AM · Restricted Project
sdesmalen added inline comments to D93639: [AArch64][SVE]Add cost model for vector reduce for scalable vector.
Fri, Jan 8, 3:21 AM · Restricted Project

Thu, Jan 7

sdesmalen added a comment to D92178: [NFC][InstructionCost] Change LoopVectorizationCostModel::getInstructionCost to return InstructionCost.

LGTM, but it's not clear to me if @ctetreau's latest comment still needs to be addressed.

Thu, Jan 7, 10:13 AM · Restricted Project
sdesmalen added a comment to D92238: [SCEVExpander] Migrate costAndCollectOperands to use InstructionCost..

I think this is conflating use-cases.
I'm not sure the entire InstructionCost should be used as a variable to track full/remaining cost over a several instructions.
Perhaps there should be a[nother] abstraction for that?

Really, we were already doing this; we just didn't have a special cost type. We could leave the cost as an integer, but then the code would just become way more verbose (unwrapping every InstructionCost after computing them). Using the metaphor of money, a cost is a quantity of money. I think it's reasonable to subtract money from a budget. I suppose we could accumulate a running cost, and compare it with the budget that remains an integer, but that would just introduce another variable for (IMO) no good reason.

Like @ctetreau I don't see why the algorithm shouldn't be using InstructionCost. The class has enough capabilities that any invalid state now propagates quite naturally without too big changes (InstructionCost behaves mostly like a scalar int that carries extra state).
I've simplified the use of *BudgetRemaining.getValue() < 0 to BudgetRemaining < 0, which hopefully makes the patch more palatable?

Thu, Jan 7, 7:51 AM · Restricted Project
sdesmalen updated the diff for D92238: [SCEVExpander] Migrate costAndCollectOperands to use InstructionCost..
  • Simplified *BudgetRemaining.getValue() < 0 to BudgetRemaining < 0.
  • Changed int Cost; to InstructionCost Cost.
  • Rebased patch.
Thu, Jan 7, 7:50 AM · Restricted Project
sdesmalen accepted D89031: [SVE] Add support to vectorize_width loop pragma for scalable vectors.

LGTM, thanks for all the changes @david-arm!

Thu, Jan 7, 7:19 AM · Restricted Project

Wed, Jan 6

sdesmalen accepted D91718: [LV] Legalize scalable VF hints.

LGTM, thanks for your work on this patch @c-rhodes!

Wed, Jan 6, 2:11 PM · Restricted Project
sdesmalen added inline comments to D89031: [SVE] Add support to vectorize_width loop pragma for scalable vectors.
Wed, Jan 6, 2:02 PM · Restricted Project
sdesmalen added a comment to D94142: [IR] Allow scalable vectors in structs to support intrinsics returning multiple values..

This change should be mentioned in the LangRef as well, because that currently specifies:

scalable vectors cannot be global variables or members of structs or arrays because their size is unknown at compile time.

Wed, Jan 6, 1:28 PM · Restricted Project
sdesmalen accepted D91806: [InstCombine] Update valueCoversEntireFragment to use TypeSize.

LGTM

Wed, Jan 6, 5:55 AM · Restricted Project, Restricted Project
sdesmalen committed rGaa280c99f708: [AArch64][SVE] Emit DWARF location expr for SVE (dbg.declare) (authored by sdesmalen).
[AArch64][SVE] Emit DWARF location expr for SVE (dbg.declare)
Wed, Jan 6, 3:46 AM
sdesmalen closed D90044: [AArch64][SVE] Emit DWARF location expr for SVE (dbg.declare).
Wed, Jan 6, 3:45 AM · Restricted Project
sdesmalen accepted D94161: [llvm][NFC] Disallow all warnings in TypeSize tests.

LGTM

Wed, Jan 6, 3:41 AM · Restricted Project
sdesmalen committed rG84a1120943a6: [LiveDebugValues] Handle spill locations with a fixed and scalable component. (authored by sdesmalen).
[LiveDebugValues] Handle spill locations with a fixed and scalable component.
Wed, Jan 6, 3:31 AM
sdesmalen closed D90046: [LiveDebugValues] Handle spill locations with a fixed and scalable component..
Wed, Jan 6, 3:31 AM · Restricted Project
sdesmalen committed rGe4cda13d5a54: Fix test failure in a7e3339f3b0eb71e43d44e6f59cc8db6a7b110bf (authored by sdesmalen).
Fix test failure in a7e3339f3b0eb71e43d44e6f59cc8db6a7b110bf
Wed, Jan 6, 2:44 AM
sdesmalen committed rGa7e3339f3b0e: [AArch64][SVE] Emit DWARF location expression for SVE stack objects. (authored by sdesmalen).
[AArch64][SVE] Emit DWARF location expression for SVE stack objects.
Wed, Jan 6, 1:55 AM
sdesmalen closed D90020: [AArch64][SVE] Emit DWARF location expression for SVE stack objects..
Wed, Jan 6, 1:55 AM · Restricted Project
sdesmalen committed rGa9f5e4375b36: [AArch64] Use faddp to implement fadd reductions. (authored by sdesmalen).
[AArch64] Use faddp to implement fadd reductions.
Wed, Jan 6, 1:38 AM
sdesmalen closed D59259: [AArch64] Use faddp to implement fadd reductions..
Wed, Jan 6, 1:37 AM · Restricted Project

Tue, Jan 5

sdesmalen added inline comments to D93639: [AArch64][SVE]Add cost model for vector reduce for scalable vector.
Tue, Jan 5, 9:58 AM · Restricted Project
sdesmalen added a comment to D90020: [AArch64][SVE] Emit DWARF location expression for SVE stack objects..

Since the prependOffsetExpression function is outside DIExpression perhaps it is worth just adding an assert that catches unknown flags?

assert((PrependFlags & ~(DerefBefore | DerefAfter | StackValue | EntryValue)) == 0 && "Unknown prepend flag");

Great suggestion, I've added the assert!

Tue, Jan 5, 4:51 AM · Restricted Project
sdesmalen updated the diff for D90020: [AArch64][SVE] Emit DWARF location expression for SVE stack objects..

Added assert.

Tue, Jan 5, 4:51 AM · Restricted Project
sdesmalen updated the diff for D59259: [AArch64] Use faddp to implement fadd reductions..

Added test with different start value for fadd reduction.

Tue, Jan 5, 1:08 AM · Restricted Project

Mon, Jan 4

sdesmalen updated the diff for D59259: [AArch64] Use faddp to implement fadd reductions..
  • Rebased patch
  • Use IMPLICIT_DEF for second source operand to faddp.
Mon, Jan 4, 8:41 AM · Restricted Project
sdesmalen added a comment to D59259: [AArch64] Use faddp to implement fadd reductions..

@sdesmalen whatever happened to this patch?

Mon, Jan 4, 6:55 AM · Restricted Project
sdesmalen added inline comments to D91718: [LV] Legalize scalable VF hints.
Mon, Jan 4, 6:42 AM · Restricted Project
sdesmalen updated the diff for D90044: [AArch64][SVE] Emit DWARF location expr for SVE (dbg.declare).
  • Rebased patch (after changes to D90020 it can use getOffsetOpcodes directly, instead of adding a new function appendOffsetExpr that does the same thing)
  • Removed unnecessary attributes from the test.
Mon, Jan 4, 4:58 AM · Restricted Project
sdesmalen added a comment to D90020: [AArch64][SVE] Emit DWARF location expression for SVE stack objects..

I'm sorry for chiming in so late here! I have a comment about the prependOffsetExpression target hook.

Thanks for the suggestion, you made a good point that this interface was error prone when new flags are added in the future. I've updated the patch and added the virtual interface getOffsetOpcodes alongside prependOffsetExpression (made non-virtual and now handles the Deref, Stack/Entry Value). Let me know if this is how you imagined it.

Mon, Jan 4, 4:55 AM · Restricted Project
sdesmalen updated the diff for D90020: [AArch64][SVE] Emit DWARF location expression for SVE stack objects..
  • Made TargetRegisterInfo::prependOffsetExpression non-virtual and changed it to handle the Prepend flags.
  • Added virtual method TargetRegisterInfo::getOffsetOpcodes, which gets called by prependOffsetExpression.
Mon, Jan 4, 4:54 AM · Restricted Project
sdesmalen added a comment to D93030: [AArch64][SVE]Add cost model for masked gather and scatter for scalable vector..

LGTM with nits addressed and TODO removed.

Mon, Jan 4, 4:43 AM · Restricted Project
sdesmalen added inline comments to D91806: [InstCombine] Update valueCoversEntireFragment to use TypeSize.
Mon, Jan 4, 12:40 AM · Restricted Project, Restricted Project

Dec 13 2020

sdesmalen added inline comments to D93132: [SVE][CodeGen] Vector + immediate addressing mode for masked gather/scatter.
Dec 13 2020, 11:15 PM · Restricted Project
sdesmalen accepted D93127: [SLPVectorizer]Migrate getEntryCost to return InstructionCost.

This change is purely mechanical. LGTM

Dec 13 2020, 10:46 PM · Restricted Project
sdesmalen added inline comments to D91957: [Support] Migrate more high level cost functions to using InstructionCost.
Dec 13 2020, 10:42 PM · Restricted Project

Dec 11 2020

sdesmalen accepted D93063: [LV] Disable epilogue vectorization for scalable VFs.

LGTM

Dec 11 2020, 8:20 AM · Restricted Project
sdesmalen added inline comments to D91718: [LV] Legalize scalable VF hints.
Dec 11 2020, 8:16 AM · Restricted Project
sdesmalen added a comment to D90020: [AArch64][SVE] Emit DWARF location expression for SVE stack objects..

On the whole, LGTM, but I'd wait a bit to see if @aprantl is happy with the explanation. (Adding debug-info group too).

@jmorse, would you be happy to formally accept the patch?

Dec 11 2020, 7:53 AM · Restricted Project
sdesmalen added inline comments to D91718: [LV] Legalize scalable VF hints.
Dec 11 2020, 3:57 AM · Restricted Project
sdesmalen added inline comments to D91718: [LV] Legalize scalable VF hints.
Dec 11 2020, 3:05 AM · Restricted Project
sdesmalen added inline comments to D93083: [InstCombine] Remove scalable vector restriction in foldSelectOpOp.
Dec 11 2020, 1:24 AM · Restricted Project

Dec 10 2020

sdesmalen added inline comments to D92094: [CostModel]Replace FixedVectorType by VectorType in costgetIntrinsicInstrCost.
Dec 10 2020, 9:22 AM · Restricted Project
sdesmalen added inline comments to D93049: [SLPVectorizer]Migrate getTreeCost() to use InstructionCost.
Dec 10 2020, 9:20 AM · Restricted Project
sdesmalen added inline comments to D93030: [AArch64][SVE]Add cost model for masked gather and scatter for scalable vector..
Dec 10 2020, 8:39 AM · Restricted Project
sdesmalen added inline comments to D92094: [CostModel]Replace FixedVectorType by VectorType in costgetIntrinsicInstrCost.
Dec 10 2020, 8:19 AM · Restricted Project
sdesmalen added inline comments to D93030: [AArch64][SVE]Add cost model for masked gather and scatter for scalable vector..
Dec 10 2020, 6:13 AM · Restricted Project
sdesmalen added inline comments to D93030: [AArch64][SVE]Add cost model for masked gather and scatter for scalable vector..
Dec 10 2020, 6:10 AM · Restricted Project
sdesmalen accepted D92819: [TruncInstCombine] Remove scalable vector restriction.

LGTM

Dec 10 2020, 12:16 AM · Restricted Project

Dec 9 2020

sdesmalen added a reviewer for D92238: [SCEVExpander] Migrate costAndCollectOperands to use InstructionCost.: ctetreau.
Dec 9 2020, 9:40 AM · Restricted Project
sdesmalen added inline comments to D90044: [AArch64][SVE] Emit DWARF location expr for SVE (dbg.declare).
Dec 9 2020, 9:27 AM · Restricted Project
sdesmalen added reviewers for D90044: [AArch64][SVE] Emit DWARF location expr for SVE (dbg.declare): djtodoro, jmorse, aprantl.
Dec 9 2020, 9:25 AM · Restricted Project
sdesmalen added a comment to D90046: [LiveDebugValues] Handle spill locations with a fixed and scalable component..

Thanks for the review @jmorse!

Dec 9 2020, 6:57 AM · Restricted Project
sdesmalen updated the diff for D90046: [LiveDebugValues] Handle spill locations with a fixed and scalable component..
  • Added extra CHECK line and TODO to test.
Dec 9 2020, 6:56 AM · Restricted Project
sdesmalen committed rGd568cff696e8: [LoopVectorizer][SVE] Vectorize a simple loop with with a scalable VF. (authored by sdesmalen).
[LoopVectorizer][SVE] Vectorize a simple loop with with a scalable VF.
Dec 9 2020, 3:26 AM
sdesmalen committed rGadc37145dec9: [LoopVectorizer] NFC: Remove unnecessary asserts that VF cannot be scalable. (authored by sdesmalen).
[LoopVectorizer] NFC: Remove unnecessary asserts that VF cannot be scalable.
Dec 9 2020, 3:26 AM
sdesmalen closed D91077: [LoopVectorizer][SVE] Vectorize a simple loop with with a scalable VF..
Dec 9 2020, 3:26 AM · Restricted Project
sdesmalen closed D91060: [LoopVectorizer] NFC: Remove unnecessary asserts that VF cannot be scalable..
Dec 9 2020, 3:26 AM · Restricted Project

Dec 8 2020

sdesmalen added a comment to D90046: [LiveDebugValues] Handle spill locations with a fixed and scalable component..

Are there any scenarios where stack slots can be accessed with different scalable offsets? For example if stack slot colouring merged two slots for vectors of different scalable size (maybe it doesn't do that). If that happened, the StackOffset object would compare differently, and we might miss a spilt value being overwritten (see llvm/test/DebugInfo/MIR/X86/live-debug-values-stack-clobber.mir for a scalar example of overwrites).

I guess that is possible. As long as both frame objects have the same Stack-ID, StackSlotColouring can merge them together. However, in that case, the sizes and offsets of those objects would both be scalable and comparing these sizes would be no different as they would be for non-scalable sizes.

Dec 8 2020, 10:19 AM · Restricted Project
sdesmalen accepted D92482: [IR] Support scalable vectors in CastInst::CreatePointerCast.

LGTM!

Dec 8 2020, 8:14 AM · Restricted Project
sdesmalen added reviewers for D92238: [SCEVExpander] Migrate costAndCollectOperands to use InstructionCost.: reames, RKSimon, fhahn, lebedev.ri, CarolineConcatto.
Dec 8 2020, 7:42 AM · Restricted Project
sdesmalen abandoned D92237: [SCEVExpander] NFCI: Change Cost type in costAndCollectOperands from int -> unsigned..

I don't like this.
I intentionally made this change from unsigned to signed back in D67318,
and then followed the same pattern when re-writing isHighCostExpansionHelper(),
because using signed is that much more easy to catch budget overrun
as opposed to basically ensuring that each subtraction doesn't cause an overflow
(or failing to do that, and having an infinite budget)

Dec 8 2020, 7:40 AM · Restricted Project
sdesmalen updated the diff for D92238: [SCEVExpander] Migrate costAndCollectOperands to use InstructionCost..
Dec 8 2020, 7:36 AM · Restricted Project
sdesmalen accepted D92230: [SVE][CodeGen] Add DAG combines for s/zext_masked_gather.

LGTM!

Dec 8 2020, 6:32 AM · Restricted Project
sdesmalen updated the diff for D91077: [LoopVectorizer][SVE] Vectorize a simple loop with with a scalable VF..

Rebased patch after D90687 and D88962 landed. This required a small change to metadata-width.ll, because the original test case tries to vectorize an induction variable, which this prototype doesn't yet support.

Dec 8 2020, 6:17 AM · Restricted Project
sdesmalen accepted D92607: [AArch64][SVE] Add lowering for llvm.maxnum|minnum for scalable type..

Thanks for adding the tests and fixing the warnings, LGTM!

Dec 8 2020, 12:55 AM · Restricted Project