Page MenuHomePhabricator

SjoerdMeijer (Sjoerd Meijer)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 26 2016, 2:17 AM (218 w, 7 h)

Recent Activity

Today

SjoerdMeijer added a comment to D77069: [ARM][MVE] Add patterns for VRHADD.

Ah, I now see that Dave commented on the other ticket, that probably needs to be addressed first...

Tue, Mar 31, 9:57 AM · Restricted Project
SjoerdMeijer added inline comments to D77069: [ARM][MVE] Add patterns for VRHADD.
Tue, Mar 31, 9:57 AM · Restricted Project
SjoerdMeijer added inline comments to D77097: [ARM] Add data gathering hint instruction.
Tue, Mar 31, 9:22 AM · Restricted Project
SjoerdMeijer accepted D77094: [ARM] Add enhanced counter virtualization system registers.

LGTM (perhaps the same nit as in the other review)

Tue, Mar 31, 9:22 AM · Restricted Project
SjoerdMeijer accepted D76991: [ARM] Add ARMv8.6 Fine Grain Traps system registers.

LGTM, with one nit that doesn't need another review.

Tue, Mar 31, 9:22 AM · Restricted Project
SjoerdMeijer updated the diff for D76838: [LV][LoopInfo] Transform counting-down loops to counting-up loop.

Rewrite using new function reverseInductionVariable in LoopInfo, see also the updated Title/Summary.

Tue, Mar 31, 8:49 AM
SjoerdMeijer added a comment to D77050: [TTI] Remove getCallCost.

Ah yes, it's getCallCost calling getIntrinsicCost, and this is about getCallCost, so agreed.

Tue, Mar 31, 7:09 AM · Restricted Project

Yesterday

SjoerdMeijer added a comment to D77050: [TTI] Remove getCallCost.

This is a tricky one, because usually it is quite simpel: if code is not used, it should be deleted. However, estimating costs of e.g. libc functions is such a glaring omission, something we definitely should be doing, but we aren't very unfortunately. I had to switch tasks before I could finish D59129, but that is just one example how we could estimate costs.... So yeah, I don't know, if this is really bothering you, we can get rid of rid, but perhaps keeping this infrastructure isn't that bad?
Perhaps this is also a good motivation to pick up D59129 again on the side...

Mon, Mar 30, 9:43 AM · Restricted Project
SjoerdMeijer added inline comments to D76998: [ARM] add ARMv8.6-A Activity monitors virtualization extension.
Mon, Mar 30, 6:27 AM · Restricted Project
SjoerdMeijer added inline comments to D76998: [ARM] add ARMv8.6-A Activity monitors virtualization extension.
Mon, Mar 30, 4:17 AM · Restricted Project

Fri, Mar 27

SjoerdMeijer added a comment to D76766: [ARM][LowOverheadLoops] DoubleWidthResult instructions canGenerateZeros.

Cheers, LGTM

Fri, Mar 27, 8:12 AM · Restricted Project
SjoerdMeijer accepted D76766: [ARM][LowOverheadLoops] DoubleWidthResult instructions canGenerateZeros.

Cheers, with some nits addressed this looks good.

Fri, Mar 27, 7:37 AM · Restricted Project
SjoerdMeijer added a comment to D76766: [ARM][LowOverheadLoops] DoubleWidthResult instructions canGenerateZeros.

A few irrelevant nits inlined, but also one question about the test.

Fri, Mar 27, 7:03 AM · Restricted Project
SjoerdMeijer committed rG401a324c5186: [LV] Refactor widenIntOrFpInduction. NFC. (authored by SjoerdMeijer).
[LV] Refactor widenIntOrFpInduction. NFC.
Fri, Mar 27, 6:30 AM
SjoerdMeijer closed D76686: [LV] widenIntOrFpInduction. NFC..
Fri, Mar 27, 6:29 AM · Restricted Project
SjoerdMeijer accepted D76762: [ARM][MVE] Add DoubleWidthResult flag.

Looks reasonable to me.

Fri, Mar 27, 6:29 AM · Restricted Project

Thu, Mar 26

SjoerdMeijer added a comment to D76838: [LV][LoopInfo] Transform counting-down loops to counting-up loop.

Thanks for commenting!

Thu, Mar 26, 11:55 AM
SjoerdMeijer accepted D76841: [ARM] Fix MVE VCMPr f16 pattern.

2-character changes that fix wrong codegen are always good changes, so have no problem directly LGTM'ing this.

Thu, Mar 26, 7:00 AM · Restricted Project
SjoerdMeijer added a comment to D76686: [LV] widenIntOrFpInduction. NFC..

Will do, and many thanks for all your reviews!

Thu, Mar 26, 5:23 AM · Restricted Project
SjoerdMeijer created D76838: [LV][LoopInfo] Transform counting-down loops to counting-up loop.
Thu, Mar 26, 5:23 AM
SjoerdMeijer updated the diff for D76686: [LV] widenIntOrFpInduction. NFC..
  • replaced ID.getStep() -> Step
  • removed the sentence on line 1899 - 1900, and
  • moved the comments on lines 1900 - 1904 to 1889.
Thu, Mar 26, 2:40 AM · Restricted Project

Wed, Mar 25

SjoerdMeijer updated the diff for D76686: [LV] widenIntOrFpInduction. NFC..

Many thanks for reviewing!
I think this has all comments addressed.

Wed, Mar 25, 1:32 PM · Restricted Project
SjoerdMeijer added inline comments to D76686: [LV] widenIntOrFpInduction. NFC..
Wed, Mar 25, 1:32 PM · Restricted Project
SjoerdMeijer accepted D76708: [ARM][LowOverheadLoops] Add horizontal reduction support.

Cheers, nice one.

Wed, Mar 25, 3:45 AM · Restricted Project
SjoerdMeijer added a comment to D76716: [ARM][MVE] Tail predicate VMAXV(unsigned) and VMAXAV.

if you change the input and example from 0x00010203 to 0x04010203, then the VMAX will also give a different result after tail-predication

Indeed! Which is why we track for our zero'd false lanes. With vmax being a horizontal operations, we check that it operates upon on registers that we know have zero'd false lanes.

Wed, Mar 25, 3:13 AM · Restricted Project
SjoerdMeijer added a comment to D76716: [ARM][MVE] Tail predicate VMAXV(unsigned) and VMAXAV.

Thanks for that example! I asked this question because I expected the vmax and vmin to behave roughly the same. In your example, if you change the input and example from 0x00010203 to 0x04010203, then the VMAX will also give a different result after tail-predication, or am I still missing something?

Wed, Mar 25, 2:08 AM · Restricted Project
SjoerdMeijer accepted D76062: [PATCH] [ARM] ARMv8.6-a command-line + BFloat16 Asm Support.

Thanks, LGTM

Wed, Mar 25, 2:08 AM · Restricted Project, Restricted Project

Tue, Mar 24

SjoerdMeijer added a comment to D76678: [SveEmitter] Add range checks for immediates and predicate patterns..

Did a first scan, looks very reasonable, just some first nits/questions inlined.

Tue, Mar 24, 1:28 PM · Restricted Project
SjoerdMeijer added inline comments to D76679: [SveEmitter] Add more immediate operand checks..
Tue, Mar 24, 1:28 PM · Restricted Project
SjoerdMeijer added a comment to D76617: [SveEmitter] Fix encoding/decoding of SVETypeFlags.

Yes you're right, the patch that I've made dependent needs this one to work properly.

Tue, Mar 24, 12:53 PM · Restricted Project
SjoerdMeijer added a comment to D76709: [Target][ARM] Adding MVE VPT Optimisation Pass.

The goal of this pass is to maximize the size of the VPT blocks created by the MVE VPT Block Insertion pass.

Tue, Mar 24, 12:53 PM · Restricted Project
SjoerdMeijer added a comment to D76716: [ARM][MVE] Tail predicate VMAXV(unsigned) and VMAXAV.

The unit test failure looks genuine, does that needs fixing?

Tue, Mar 24, 11:49 AM · Restricted Project
SjoerdMeijer added inline comments to D76708: [ARM][LowOverheadLoops] Add horizontal reduction support.
Tue, Mar 24, 11:49 AM · Restricted Project
SjoerdMeijer accepted D76683: [ARM][MVE] Add HorizontalReduction flag.

LGTM

Tue, Mar 24, 10:44 AM · Restricted Project
SjoerdMeijer updated the diff for D76686: [LV] widenIntOrFpInduction. NFC..

Thanks for taking a look @fhahn ! Comments addressed.

Tue, Mar 24, 8:34 AM · Restricted Project
SjoerdMeijer created D76686: [LV] widenIntOrFpInduction. NFC..
Tue, Mar 24, 4:48 AM · Restricted Project
SjoerdMeijer added a comment to D76617: [SveEmitter] Fix encoding/decoding of SVETypeFlags.

Patches with functional changes but without tests are a bit "suspicious". In this case, I see it might be tricky. You could argue that it should be incorporated in the patch that includes the tests, so we can see what's happening. But perhaps separating this out is cleaner, if the other patch is big. But can you at least make the next patch dependent on this one, so we know where this is used?

Tue, Mar 24, 3:44 AM · Restricted Project

Mon, Mar 23

SjoerdMeijer accepted D76629: [ARM] MVE VMOV.i64.

looks like a good fix to me.

Mon, Mar 23, 12:33 PM
SjoerdMeijer accepted D76235: [ARM][LowOverheadLoops] Add checks for narrowing.

Cheers, looks good to me.

Mon, Mar 23, 12:33 PM · Restricted Project
SjoerdMeijer accepted D76608: [ARM][MVE] Add target flag for narrowing insts.

Thanks, LGTM

Mon, Mar 23, 10:21 AM · Restricted Project
SjoerdMeijer added a comment to D76608: [ARM][MVE] Add target flag for narrowing insts.

Looks like a good direction to me.

Mon, Mar 23, 8:41 AM · Restricted Project

Fri, Mar 20

SjoerdMeijer accepted D76238: [SveEmitter] Implement builtins for contiguous loads/stores.

Looks good to me. Please wait a day in case Eli has more comments.

Fri, Mar 20, 8:05 AM · Restricted Project

Thu, Mar 19

SjoerdMeijer added a comment to D76235: [ARM][LowOverheadLoops] Add checks for narrowing.

I am now up to date with this, and don't have any questions about the logic, that looks good.
I would like to go over it once more when (some of) the nits have been addressed as I would like to get an overview how things read and look as this introduces quite a few new things and concepts.

Thu, Mar 19, 6:26 AM · Restricted Project
SjoerdMeijer added a comment to D76062: [PATCH] [ARM] ARMv8.6-a command-line + BFloat16 Asm Support.

Besides the irrelevant formatting nits, one minor question about the clang test.

Thu, Mar 19, 5:21 AM · Restricted Project, Restricted Project
SjoerdMeijer added inline comments to D76238: [SveEmitter] Implement builtins for contiguous loads/stores.
Thu, Mar 19, 5:21 AM · Restricted Project
SjoerdMeijer added inline comments to D76235: [ARM][LowOverheadLoops] Add checks for narrowing.
Thu, Mar 19, 3:44 AM · Restricted Project
SjoerdMeijer added a comment to D76238: [SveEmitter] Implement builtins for contiguous loads/stores.

Some nits inlined

Thu, Mar 19, 3:44 AM · Restricted Project

Wed, Mar 18

SjoerdMeijer added a comment to D76077: [ARM] Add __bf16 as new Bfloat16 C Type.

That sounds good and that seems to address the feedback given here, which I agree with. Just wanted to briefly add that while it already looks like most interested people are on this ticket, perhaps it good to also share the plan and design on the cfe dev list for visibility.

Wed, Mar 18, 5:56 AM · Restricted Project
SjoerdMeijer added inline comments to D76062: [PATCH] [ARM] ARMv8.6-a command-line + BFloat16 Asm Support.
Wed, Mar 18, 4:18 AM · Restricted Project, Restricted Project
SjoerdMeijer added inline comments to D76235: [ARM][LowOverheadLoops] Add checks for narrowing.
Wed, Mar 18, 2:08 AM · Restricted Project

Mon, Mar 16

SjoerdMeijer added a comment to D75533: [ARM][LowOverheadLoops] Handle reductions.

Big patch: this is just a first scan of the code, and a first round of nits. Now going to look again, to let things sink in.

Mon, Mar 16, 6:59 AM · Restricted Project

Thu, Mar 12

SjoerdMeijer added a comment to rG825235c140e7: Revert "[Sema] Use the canonical type in function isVector".

I'm still not sure why __fp16, which is a storage-only type, is used for the element type of float16x4_t if we want to avoid promotion to a float vector type.

Thu, Mar 12, 4:30 AM

Wed, Mar 11

SjoerdMeijer accepted D75861: [SVE] Generate overloaded functions for ACLE intrinsics..

With the nit addressed, this LGTM

Wed, Mar 11, 4:30 AM · Restricted Project

Tue, Mar 10

SjoerdMeijer added inline comments to D75861: [SVE] Generate overloaded functions for ACLE intrinsics..
Tue, Mar 10, 1:39 PM · Restricted Project
SjoerdMeijer accepted D75470: [SVE] Auto-generate builtins and header for svld1..

Cheers, I think this looks very reasonable.

Tue, Mar 10, 6:24 AM · Restricted Project
SjoerdMeijer accepted D75452: [ARM][MVE] Validate tail predication values.

LGTM, with one nit inline.

Tue, Mar 10, 3:02 AM · Restricted Project

Mon, Mar 9

SjoerdMeijer added a comment to D75452: [ARM][MVE] Validate tail predication values.

was just wondering about more corner cases for "negative" tests: do we have test where lanes are swapped (if that makes sense)?

Mon, Mar 9, 3:40 PM · Restricted Project
SjoerdMeijer added inline comments to D75470: [SVE] Auto-generate builtins and header for svld1..
Mon, Mar 9, 2:35 PM · Restricted Project
SjoerdMeijer committed rGe32f8ef9277d: Follow up of 3d9a0445cce3, clang driver defaulting to -fno-common (authored by SjoerdMeijer).
Follow up of 3d9a0445cce3, clang driver defaulting to -fno-common
Mon, Mar 9, 2:04 PM
SjoerdMeijer committed rG3d9a0445cce3: Recommit #2 "[Driver] Default to -fno-common for all targets" (authored by SjoerdMeijer).
Recommit #2 "[Driver] Default to -fno-common for all targets"
Mon, Mar 9, 12:58 PM
SjoerdMeijer committed rGf35d112efdb3: Revert "Recommit "[Driver] Default to -fno-common for all targets"" (authored by SjoerdMeijer).
Revert "Recommit "[Driver] Default to -fno-common for all targets""
Mon, Mar 9, 3:44 AM
SjoerdMeijer added a reverting change for rG2c36c23f3476: Recommit "[Driver] Default to -fno-common for all targets": rGf35d112efdb3: Revert "Recommit "[Driver] Default to -fno-common for all targets"".
Mon, Mar 9, 3:44 AM
SjoerdMeijer committed rG2c36c23f3476: Recommit "[Driver] Default to -fno-common for all targets" (authored by SjoerdMeijer).
Recommit "[Driver] Default to -fno-common for all targets"
Mon, Mar 9, 3:12 AM

Thu, Mar 5

SjoerdMeijer accepted D75669: [ARM][MVE] Enable VMOVN for tail predication.

LGTM

Thu, Mar 5, 12:39 PM · Restricted Project
SjoerdMeijer closed D75557: [test-suite] Add -fcommon.

Committed in: https://github.com/llvm/llvm-test-suite/commit/2af3f724f9c857587cecbc03f7f88f605d739ea2

Thu, Mar 5, 12:39 PM
SjoerdMeijer added a comment to D75520: [compiler-rt] Fix tests after defaulting to -fno-common. NFC..

I wanted to include these test changes in the recommit, but perhaps it's nice to commit this separately (as this this a change in a different component).
So wanted to check if we are happy with this?

Thu, Mar 5, 12:07 PM · Restricted Project
SjoerdMeijer updated subscribers of D75470: [SVE] Auto-generate builtins and header for svld1..

Adding @simon_tatham in case he feels wants to have a look too.

Thu, Mar 5, 7:08 AM · Restricted Project
SjoerdMeijer added inline comments to D75470: [SVE] Auto-generate builtins and header for svld1..
Thu, Mar 5, 6:01 AM · Restricted Project
SjoerdMeijer accepted D75667: [ARM][MVE] Enable *SHRN* for tail predication.

Agreed, they don't swap lanes. They can write to only bottom/top halfs, but that's fine. So looks like a straightforward change to me.

Thu, Mar 5, 2:46 AM · Restricted Project
SjoerdMeijer added inline comments to D75452: [ARM][MVE] Validate tail predication values.
Thu, Mar 5, 2:46 AM · Restricted Project

Wed, Mar 4

SjoerdMeijer added a comment to D75470: [SVE] Auto-generate builtins and header for svld1..

Big patch, I only did a first scan, but looks very reasonable in general. Just a first round of nits and 2 questions.

Wed, Mar 4, 8:24 AM · Restricted Project
SjoerdMeijer updated the diff for D75520: [compiler-rt] Fix tests after defaulting to -fno-common. NFC..

Thanks for that solution, have copied it to the test file.

Wed, Mar 4, 2:53 AM · Restricted Project

Tue, Mar 3

SjoerdMeijer added a comment to D75056: [Driver] Default to -fno-common for all targets.

Patch for the test-suite: D75557

Tue, Mar 3, 12:37 PM · Restricted Project
SjoerdMeijer created D75557: [test-suite] Add -fcommon.
Tue, Mar 3, 12:37 PM
SjoerdMeijer added a comment to D75520: [compiler-rt] Fix tests after defaulting to -fno-common. NFC..

Thanks James, I will prepare a new revision!
The test-suite needs porting too (see comment in D75056) and am first looking into that, after that I will return to this.

Tue, Mar 3, 9:39 AM · Restricted Project
SjoerdMeijer added a comment to D75056: [Driver] Default to -fno-common for all targets.

Ha, these test-suite apps fail with multiple definition of ... link errors, so actually require a little bit of porting! :-)
But I think I will prepare a patch that adds -fcommon to their makefiles, as I don't want to change too many things at the same time.

Tue, Mar 3, 9:39 AM · Restricted Project
SjoerdMeijer added a comment to D75056: [Driver] Default to -fno-common for all targets.

Reverted in: https://reviews.llvm.org/rG4e363563fa14

Tue, Mar 3, 6:56 AM · Restricted Project
SjoerdMeijer added a comment to D75520: [compiler-rt] Fix tests after defaulting to -fno-common. NFC..

I probably forgot to add why these tests started failing!

Tue, Mar 3, 6:40 AM · Restricted Project
SjoerdMeijer created D75520: [compiler-rt] Fix tests after defaulting to -fno-common. NFC..
Tue, Mar 3, 6:38 AM · Restricted Project
SjoerdMeijer committed rG4e363563fa14: Revert "[Driver] Default to -fno-common for all targets" (authored by SjoerdMeijer).
Revert "[Driver] Default to -fno-common for all targets"
Tue, Mar 3, 2:11 AM
SjoerdMeijer added a reverting change for rG0a9fc9233e17: [Driver] Default to -fno-common for all targets: rG4e363563fa14: Revert "[Driver] Default to -fno-common for all targets".
Tue, Mar 3, 2:11 AM
SjoerdMeijer committed rG0a9fc9233e17: [Driver] Default to -fno-common for all targets (authored by SjoerdMeijer).
[Driver] Default to -fno-common for all targets
Tue, Mar 3, 1:44 AM
SjoerdMeijer closed D75056: [Driver] Default to -fno-common for all targets.
Tue, Mar 3, 1:43 AM · Restricted Project

Mon, Mar 2

SjoerdMeijer added a comment to D75056: [Driver] Default to -fno-common for all targets.

Thanks all for the reviews and help.
I will fix these things and do a last rebase/check before committing this tomorrow.

Mon, Mar 2, 9:29 AM · Restricted Project
SjoerdMeijer accepted D75167: [RDA][ARM] collectKilledOperands across multiple blocks.

Nice optimisation, looks very reasonable to me: LGTM

Mon, Mar 2, 7:53 AM · Restricted Project
SjoerdMeijer accepted D75245: [ARM][RDA] Allow multiple killed users.

LGTM

Mon, Mar 2, 7:24 AM · Restricted Project
SjoerdMeijer added a comment to D75056: [Driver] Default to -fno-common for all targets.

Friendly ping, and just checking: are we happy with this? Good to go?

Mon, Mar 2, 2:03 AM · Restricted Project

Feb 28 2020

SjoerdMeijer added a comment to D75245: [ARM][RDA] Allow multiple killed users.

Looks like this needs a rebase?

Feb 28 2020, 7:44 AM · Restricted Project
SjoerdMeijer accepted D75185: [RDA] Track implicit-defs.

Thanks, LGTM

Feb 28 2020, 2:58 AM · Restricted Project
SjoerdMeijer added inline comments to D75185: [RDA] Track implicit-defs.
Feb 28 2020, 2:08 AM · Restricted Project

Feb 27 2020

SjoerdMeijer committed rG13db7490fa67: [AArch64] Peephole optimization: merge AND and TST instructions (authored by SjoerdMeijer).
[AArch64] Peephole optimization: merge AND and TST instructions
Feb 27 2020, 9:47 AM
SjoerdMeijer added a comment to D75185: [RDA] Track implicit-defs.

Thanks for that! That greatly helps readability!

Feb 27 2020, 7:00 AM · Restricted Project
SjoerdMeijer added inline comments to D75185: [RDA] Track implicit-defs.
Feb 27 2020, 5:30 AM · Restricted Project
SjoerdMeijer updated the diff for D75056: [Driver] Default to -fno-common for all targets.

Are there any tests remaining that check that with -fcommon, IR generation creates "common" variables, now that all these tests have been modified?

Feb 27 2020, 5:14 AM · Restricted Project
SjoerdMeijer closed D71701: [AArch64] Peephole optimization. Merge AND and TST instructions.

That should happen automatically when you put the tag Differential Revision: ... in the commit message.
It doesn't look like a made spell mistake in it, so am a bit puzzled why this didn't happen.... but anyway, it's closed now.

Feb 27 2020, 5:05 AM · Restricted Project
SjoerdMeijer added a comment to D71701: [AArch64] Peephole optimization. Merge AND and TST instructions.

Sure, no problem, this is committed in: https://github.com/llvm/llvm-project/commit/13db7490fa67e22605dec4ab824121230b0fd928

Feb 27 2020, 1:35 AM · Restricted Project

Feb 26 2020

SjoerdMeijer updated the diff for D75056: [Driver] Default to -fno-common for all targets.

Removed the CHECK-NOTs from some tests as suggested.

Feb 26 2020, 12:17 PM · Restricted Project
SjoerdMeijer added a comment to D71701: [AArch64] Peephole optimization. Merge AND and TST instructions.

Yep, those are the instructions to request commit access, which might be good to request if you plan to do more llvm work. I guess that will probably take a few days. And if you're not in a hurry with getting this committed, you could wait for your commit access and then commit this yourself. But I would of course be happy to commit this on your behalf, which I will then do tomorrow morning, just let me know what you prefer.

Feb 26 2020, 11:59 AM · Restricted Project
SjoerdMeijer updated the diff for D75056: [Driver] Default to -fno-common for all targets.

minor test update

Feb 26 2020, 9:50 AM · Restricted Project
SjoerdMeijer updated the diff for D75056: [Driver] Default to -fno-common for all targets.

Thanks for catching that. Now it shows more changes in tests, so updated a bunch of tests.

Feb 26 2020, 9:50 AM · Restricted Project