Page MenuHomePhabricator

SjoerdMeijer (Sjoerd Meijer)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 26 2016, 2:17 AM (160 w, 4 d)

Recent Activity

Thu, Feb 21

SjoerdMeijer accepted D58452: [ARM] Negative constants mishandled in ARM CGP.

LGTM

Thu, Feb 21, 12:44 AM · Restricted Project

Tue, Feb 19

SjoerdMeijer accepted D58306: [AArch64] Change size suffix for FP16FML intrinsics..

The ACLE has been updated and a new version with change included will be released soon.

Tue, Feb 19, 5:08 AM · Restricted Project, Restricted Project

Sun, Feb 17

SjoerdMeijer added a comment to D58306: [AArch64] Change size suffix for FP16FML intrinsics..

I am discussing this with our GCC team as we would like both Clang/GCC implementation to be the same. But you're right that _f16 looks like to be the more consistent choice. I will let you know as soon I know more.

Sun, Feb 17, 12:11 PM · Restricted Project, Restricted Project

Fri, Feb 15

SjoerdMeijer added inline comments to D53633: [AArch64] Implement FP16FML intrinsics.
Fri, Feb 15, 6:57 AM · Restricted Project
SjoerdMeijer added inline comments to D53633: [AArch64] Implement FP16FML intrinsics.
Fri, Feb 15, 2:37 AM · Restricted Project

Thu, Feb 14

SjoerdMeijer accepted D57686: [ARM CGP] Fix ConvertTruncs.

Thanks, LGTM

Thu, Feb 14, 5:45 AM · Restricted Project

Wed, Feb 13

SjoerdMeijer accepted D58176: [ARM] Ensure we update the correct flags in the peephole optimiser.

LGTM, with one nit inline.
Perhaps wait a day in case someone else has comments.

Wed, Feb 13, 6:45 AM · Restricted Project
SjoerdMeijer added inline comments to D57686: [ARM CGP] Fix ConvertTruncs.
Wed, Feb 13, 6:06 AM · Restricted Project

Mon, Feb 11

SjoerdMeijer committed rG150ccb889e9e: [ARM] LoadStoreOptimizer: reoder limit (authored by SjoerdMeijer).
[ARM] LoadStoreOptimizer: reoder limit
Mon, Feb 11, 1:38 AM
SjoerdMeijer committed rL353678: [ARM] LoadStoreOptimizer: reoder limit.
[ARM] LoadStoreOptimizer: reoder limit
Mon, Feb 11, 1:37 AM
SjoerdMeijer closed D57954: [ARM] LoadStoreOptimizer: reoder limit.
Mon, Feb 11, 1:37 AM · Restricted Project
SjoerdMeijer added a comment to D57954: [ARM] LoadStoreOptimizer: reoder limit.

Thanks Eli, also for your thoughts on this:

Mon, Feb 11, 1:27 AM · Restricted Project
SjoerdMeijer committed rG0cc50c6b87c4: [ARM] LoadStoreOptimizer: just a clean-up. NFC. (authored by SjoerdMeijer).
[ARM] LoadStoreOptimizer: just a clean-up. NFC.
Mon, Feb 11, 12:48 AM
SjoerdMeijer committed rL353670: [ARM] LoadStoreOptimizer: just a clean-up. NFC..
[ARM] LoadStoreOptimizer: just a clean-up. NFC.
Mon, Feb 11, 12:48 AM
SjoerdMeijer closed D57955: [ARM] LoadStoreOptimizer: just a clean-up. NFC..
Mon, Feb 11, 12:47 AM · Restricted Project

Fri, Feb 8

SjoerdMeijer updated the summary of D57954: [ARM] LoadStoreOptimizer: reoder limit.
Fri, Feb 8, 8:45 AM · Restricted Project
SjoerdMeijer created D57955: [ARM] LoadStoreOptimizer: just a clean-up. NFC..
Fri, Feb 8, 8:43 AM · Restricted Project
SjoerdMeijer created D57954: [ARM] LoadStoreOptimizer: reoder limit.
Fri, Feb 8, 8:41 AM · Restricted Project
SjoerdMeijer accepted D57577: Make predefined FLT16 macros conditional on support for the type.

Looks okay to me, with one nit inline.

Fri, Feb 8, 5:38 AM · Restricted Project, Restricted Project

Thu, Jan 31

SjoerdMeijer committed rL352737: [ARM] Thumb2: ConstantMaterializationCost.
[ARM] Thumb2: ConstantMaterializationCost
Thu, Jan 31, 12:40 AM
SjoerdMeijer closed D57327: [ARM] Thumb2: ConstantMaterializationCost.
Thu, Jan 31, 12:40 AM
SjoerdMeijer added inline comments to D57327: [ARM] Thumb2: ConstantMaterializationCost.
Thu, Jan 31, 12:19 AM
SjoerdMeijer committed rL352736: [SelectionDAG] Codesize: don't expand SHIFT to SHIFT_PARTS.
[SelectionDAG] Codesize: don't expand SHIFT to SHIFT_PARTS
Thu, Jan 31, 12:09 AM
SjoerdMeijer closed D57386: [SelectionDAG] Codesize: don't expand SHIFT to SHIFT_PARTS.
Thu, Jan 31, 12:09 AM

Wed, Jan 30

SjoerdMeijer updated the diff for D57386: [SelectionDAG] Codesize: don't expand SHIFT to SHIFT_PARTS.

It probably should be shouldExpandShift or something?

Wed, Jan 30, 5:38 AM
SjoerdMeijer updated the diff for D57386: [SelectionDAG] Codesize: don't expand SHIFT to SHIFT_PARTS.

Thanks for reviewing!

Wed, Jan 30, 5:13 AM
SjoerdMeijer updated the diff for D57327: [ARM] Thumb2: ConstantMaterializationCost.

Maybe put the tests into test/CodeGen/ARM/subtarget-no-movt.ll? We have some similar tests there from https://reviews.llvm.org/D23090 .

Wed, Jan 30, 2:52 AM

Tue, Jan 29

SjoerdMeijer created D57386: [SelectionDAG] Codesize: don't expand SHIFT to SHIFT_PARTS.
Tue, Jan 29, 7:27 AM
SjoerdMeijer updated the diff for D57327: [ARM] Thumb2: ConstantMaterializationCost.

Addressed comments.

Tue, Jan 29, 1:25 AM
SjoerdMeijer added inline comments to D57327: [ARM] Thumb2: ConstantMaterializationCost.
Tue, Jan 29, 1:10 AM

Mon, Jan 28

SjoerdMeijer created D57327: [ARM] Thumb2: ConstantMaterializationCost.
Mon, Jan 28, 7:19 AM

Fri, Jan 25

SjoerdMeijer added inline comments to D57188: Disable _Float16 for non ARM/SPIR Targets.
Fri, Jan 25, 2:20 AM

Jan 23 2019

SjoerdMeijer accepted D57041: [ARM][CGP] Check trunc value size before replacing.

Thanks, LGTM

Jan 23 2019, 1:07 AM

Jan 22 2019

SjoerdMeijer added a comment to D57041: [ARM][CGP] Check trunc value size before replacing.

Was just getting up to speed with this again; just a first round of nits before I start looking at the actual functional change.

Jan 22 2019, 2:59 AM

Jan 21 2019

SjoerdMeijer added a comment to D56281: [DAGCombiner] reduce buildvec of zexted extracted element to shuffle.

Many thanks for your speedy responses and for fixing this!

Jan 21 2019, 11:23 AM
SjoerdMeijer added a comment to rL351754: [AArch64] add more tests for buildvec to shuffle transform; NFC.

Many thanks for fixing this. I am going to look at this tomorrow morning.

Jan 21 2019, 11:23 AM
SjoerdMeijer added a comment to D56281: [DAGCombiner] reduce buildvec of zexted extracted element to shuffle.

Hello. I am investigating a crash and this assertion failure on AArch64:

Jan 21 2019, 8:04 AM

Jan 14 2019

SjoerdMeijer accepted D56616: [AArch64] Explicitly use v1i64 type for llvm.aarch64.neon.abs.i64 ..

LGTM

Jan 14 2019, 1:13 PM
SjoerdMeijer added inline comments to D56616: [AArch64] Explicitly use v1i64 type for llvm.aarch64.neon.abs.i64 ..
Jan 14 2019, 7:17 AM
SjoerdMeijer added a comment to D56596: Enable fma formation for fp16 on x86 and aarch64.

The AArch64 side LGTM. Also adding Oliver and Sjoerd as they worked on FP16 side as well, in case they have any thoughts.

Jan 14 2019, 6:55 AM

Jan 9 2019

SjoerdMeijer accepted D56296: [AArch64] Fix operation actions for FP16 vector intrinsics.

LGTM

Jan 9 2019, 12:48 AM

Jan 8 2019

SjoerdMeijer accepted D56255: [ARM] Size reduce teq to eors.

Thanks, looks okay to me. Perhaps wait a day with committing to give people one more chance to comment.

Jan 8 2019, 2:11 AM

Jan 7 2019

SjoerdMeijer added a comment to D56255: [ARM] Size reduce teq to eors.

Now that we have MIR tests, do we still need the added .ll test?

Jan 7 2019, 6:50 AM
SjoerdMeijer updated subscribers of D56255: [ARM] Size reduce teq to eors.

This looks fine to me, but it's the first time I look at this file and table. Perhaps @dmgreen or @efriedma can have a look too.

Jan 7 2019, 1:06 AM

Dec 3 2018

SjoerdMeijer committed rL348110: [ARM] FP16: support vld1.16 for vector loads with post-increment.
[ARM] FP16: support vld1.16 for vector loads with post-increment
Dec 3 2018, 12:29 AM
SjoerdMeijer closed D55112: [ARM] FP16: select vld1.16 for vector loads with post-increment.
Dec 3 2018, 12:29 AM

Nov 30 2018

SjoerdMeijer created D55112: [ARM] FP16: select vld1.16 for vector loads with post-increment.
Nov 30 2018, 2:57 AM
SjoerdMeijer added a comment to D55059: [ARM] FP16: constant initialised v4f16 and v8f16 vectors.

Thanks for taking a look!

Nov 30 2018, 1:43 AM
SjoerdMeijer committed rL347965: [ARM] Don't expand sdiv when optimising for minsize.
[ARM] Don't expand sdiv when optimising for minsize
Nov 30 2018, 12:17 AM
SjoerdMeijer closed D54546: [ARM] Don't expand sdiv when optimising for minsize.
Nov 30 2018, 12:17 AM
SjoerdMeijer added a comment to D54546: [ARM] Don't expand sdiv when optimising for minsize.

Thanks!

Nov 30 2018, 12:00 AM

Nov 29 2018

SjoerdMeijer created D55059: [ARM] FP16: constant initialised v4f16 and v8f16 vectors.
Nov 29 2018, 9:10 AM
SjoerdMeijer added a comment to D54546: [ARM] Don't expand sdiv when optimising for minsize.

friendly ping

Nov 29 2018, 9:00 AM

Nov 27 2018

SjoerdMeijer updated the diff for D54546: [ARM] Don't expand sdiv when optimising for minsize.

Bail earlier if minsize or hwdiv is not enabled.

Nov 27 2018, 7:03 AM
SjoerdMeijer added a comment to D54769: [FileCheck] New option -warn.

Ok, nice, looks like we actually agree: we want this to be errors, not warnings. Perhaps I conflated things too, and this was likely caused for practical reasons: how do we get all tests error free so that we the we can enable the error checking?

Nov 27 2018, 1:50 AM

Nov 26 2018

SjoerdMeijer added a comment to D54769: [FileCheck] New option -warn.

And I do remember a possibly valid use case for emitting a diagnostic. I've seen quite a few tests with names like test-pr12345.ll, that were reproducers of compiler crashes, but only contained CHECK-LABELS. I called them dubious tests before (but we don't want to use that term), because these tests are happy when a functions compiles, but don't tests anything whereas they probably should. There are probably a few exceptions where only CHECK-LABELs are okay, so I think a warning along the lines of "you're only checking labels, is that what you really want?" should be fair?

Nov 26 2018, 4:51 PM
SjoerdMeijer added a comment to D54769: [FileCheck] New option -warn.

So, I'm not convinced that we can have a diagnostic for your case 3 that would really be meaningful and useful.

Nov 26 2018, 4:21 PM
SjoerdMeijer added a comment to D54769: [FileCheck] New option -warn.

Looks like we are unhappy with FileCheck for many and different reasons:

  1. we want to error for overlapping dag checks,
  2. we want to error for the example in this diff: 1 but not all prefixes are checked,
  3. we want a diagnostic for my motivating example in D53710: only labels are checked (check-prefixes are renamed/refactored/misspelled, but not the tags in tests accordingly)
  4. CHECK-LABEL is confusing and probably mostly misunderstood
Nov 26 2018, 3:55 PM
SjoerdMeijer added a comment to D54769: [FileCheck] New option -warn.

Sounds like that could be an error, rather than a warning, perhaps?

Nov 26 2018, 1:00 PM

Nov 23 2018

SjoerdMeijer updated the diff for D54546: [ARM] Don't expand sdiv when optimising for minsize.
  • Don't create div libcalls in ARM mode,
  • Bail for SREM instructions. This wasn't checked, and resulted in a crash. Thanks to @dmgreen for pointing this out. Will follow up on this.
Nov 23 2018, 7:17 AM
SjoerdMeijer added a comment to D54769: [FileCheck] New option -warn.

fyi: as a first exercise, I found some (real) issues and fixed/committed them in https://reviews.llvm.org/rL347487

Nov 23 2018, 3:09 AM
SjoerdMeijer committed rL347487: [ARM][NFC] codegen tests cleanup: remove dangling check prefixes.
[ARM][NFC] codegen tests cleanup: remove dangling check prefixes
Nov 23 2018, 2:11 AM
SjoerdMeijer closed D54842: [ARM][NFC] codegen tests cleanup: remove dangling check prefixes.
Nov 23 2018, 2:11 AM
SjoerdMeijer accepted D54790: [ARM] Prevent parallel macs for unsigned values.

LGTM

Nov 23 2018, 2:05 AM
SjoerdMeijer created D54842: [ARM][NFC] codegen tests cleanup: remove dangling check prefixes.
Nov 23 2018, 1:16 AM

Nov 22 2018

SjoerdMeijer added inline comments to D54546: [ARM] Don't expand sdiv when optimising for minsize.
Nov 22 2018, 8:41 AM
SjoerdMeijer updated the diff for D54769: [FileCheck] New option -warn.

I decided to continue here, and implemented the diagnostic that Paul suggested in the other ticket, because it's small and a nice demonstrator for this new option:

Nov 22 2018, 6:00 AM

Nov 21 2018

SjoerdMeijer updated the diff for D54769: [FileCheck] New option -warn.

This is small enough that I'd prefer to see it incorporated into the patch with the "real" warning, rather than have this temporary placeholder diagnostic.

Nov 21 2018, 8:47 AM
SjoerdMeijer updated the diff for D54769: [FileCheck] New option -warn.

Is the purpose of this warning purely for testing your -warn implementation?

Nov 21 2018, 8:08 AM
SjoerdMeijer updated the diff for D54546: [ARM] Don't expand sdiv when optimising for minsize.

Thanks for reviewing!

  • removed the unnecessary assert
  • added support for ARM mode, and tests.
Nov 21 2018, 7:38 AM
SjoerdMeijer added inline comments to D54546: [ARM] Don't expand sdiv when optimising for minsize.
Nov 21 2018, 6:03 AM
SjoerdMeijer updated the diff for D54769: [FileCheck] New option -warn.

Thanks for taking a look!

Nov 21 2018, 1:31 AM

Nov 20 2018

SjoerdMeijer created D54769: [FileCheck] New option -warn.
Nov 20 2018, 12:36 PM
SjoerdMeijer added a comment to D53710: [FileCheck] Warn if a prefix is only used in LABEL checks.

I'm back :-) Catching up on the last messages.... I will post a patch for review soon that adds an option -warn to FileCheck, and will do some initial plumbing for that. If I understood correctly, it looks like we think that is best. After that initial plumbing, I will then return to this patch. Is that a plan?

Nov 20 2018, 5:38 AM
SjoerdMeijer updated the diff for D54546: [ARM] Don't expand sdiv when optimising for minsize.

While looking what X86 as doing (and why it was doing the right thing), I noticed that I should just implement TargetLowering::BuildSDIVPow2. So that's what I am doing now, and thus it's no longer a DAGCombine change but just an ARM one.
Different tests and targets have been added, and a check for larger constants.

Nov 20 2018, 4:58 AM

Nov 19 2018

SjoerdMeijer accepted D54515: [ARM] Replace trunc for switch as a sink.

Thanks, looks okay to me.

Nov 19 2018, 2:50 AM

Nov 16 2018

SjoerdMeijer added a comment to D54546: [ARM] Don't expand sdiv when optimising for minsize.

A little bit of progress:

Nov 16 2018, 9:37 AM

Nov 14 2018

SjoerdMeijer retitled D54546: [ARM] Don't expand sdiv when optimising for minsize from [ARM] Don't expand sdiv to [DAGCombiner] Don't expand sdiv when optimising for minsize.
Nov 14 2018, 3:35 PM
SjoerdMeijer added a comment to D54546: [ARM] Don't expand sdiv when optimising for minsize.

Thanks for commenting!
Yes, this should save 2 bytes.
I will check tomorrow:

  • about the SDIV legality, and where exactly the SDIV is expanded when it is not supported in HW. I see that sdiv is expanded very early if it is not available, immediately after the initial SDAG, and this code in visitSDIVLike does not get triggered. But again, will double check where exactly the magic happens. I've noticed that at least some tests are in place for other targets: the minsize check is no, the !VT.isVector() is required for some X86 and AArch64 tests, and without it the (knock on) effect is that the vector operations get scalarized and these tests fail, but I don't think this is related the legality of SDIV.
  • I missed this: "On Thumbv8, if you're dividing by a power of two greater than 128, I think you save zero byte", so will address that.
  • what exactly the benefit is on some other targets.
Nov 14 2018, 3:30 PM
SjoerdMeijer created D54546: [ARM] Don't expand sdiv when optimising for minsize.
Nov 14 2018, 2:33 PM
SjoerdMeijer added inline comments to D54515: [ARM] Replace trunc for switch as a sink.
Nov 14 2018, 6:46 AM
SjoerdMeijer added a comment to D54515: [ARM] Replace trunc for switch as a sink.

Just a very quick first pass....some nitpicks:

Nov 14 2018, 1:21 AM

Nov 13 2018

SjoerdMeijer added a comment to D53710: [FileCheck] Warn if a prefix is only used in LABEL checks.

Sorry, just a quick message: I'm a few days out of office, and will get back to this end of this week.

Nov 13 2018, 3:24 PM

Nov 9 2018

SjoerdMeijer added a comment to D53710: [FileCheck] Warn if a prefix is only used in LABEL checks.

Agreed with all previous comments! And yes, I messed up my last lit test example :-( and I looked at yours and Paul's, and agree. But anyway, we have a plan...

Nov 9 2018, 7:25 AM
SjoerdMeijer accepted D54308: [ARM] Don't promote i1 types in ARM CGP.

Looks like a straightforward fix to me.

Nov 9 2018, 6:37 AM
SjoerdMeijer accepted D54108: [ARM] Enable mixed types in ARM CGP.

LGTM

Nov 9 2018, 1:16 AM
SjoerdMeijer accepted D54254: [ARM] Reorganise some functionality in ARMParallelDSP.

LGTM

Nov 9 2018, 12:11 AM

Nov 8 2018

SjoerdMeijer added a comment to D53710: [FileCheck] Warn if a prefix is only used in LABEL checks.

Hi Paul and Joel, thanks for your help here, appreciate it! I thought this was
a nice little addition, low-hanging fruit, but it looks like it turns into
proper project! :-) Also from our discussions, it looks like we have not just
one but a few problems. If I try to summarise them:

Nov 8 2018, 3:25 AM

Nov 7 2018

SjoerdMeijer accepted D54192: [ARM] Fix CPSR liveness in tMOVCCr_pseudo lowering..

Looks very reasonable to me.

Nov 7 2018, 5:22 AM
SjoerdMeijer added a comment to D53710: [FileCheck] Warn if a prefix is only used in LABEL checks.

AMDGPU does this a lot intentionally for a shared label prefix and unique per run like ones for different subtargets

Nov 7 2018, 2:49 AM

Nov 6 2018

SjoerdMeijer added a comment to D53710: [FileCheck] Warn if a prefix is only used in LABEL checks.

Hi Joel, thanks for taking another look!

Nov 6 2018, 8:16 AM
SjoerdMeijer added a comment to D53710: [FileCheck] Warn if a prefix is only used in LABEL checks.

Friendly ping.

Nov 6 2018, 5:43 AM
SjoerdMeijer added inline comments to D54108: [ARM] Enable mixed types in ARM CGP.
Nov 6 2018, 1:32 AM

Nov 5 2018

SjoerdMeijer accepted D54094: [ARM] Turn assert into condition in ARMCGP.

Looks like a straightforward fix/addition to me.

Nov 5 2018, 3:04 AM
SjoerdMeijer accepted D54032: [ARM][ARMCGP] Remove unecessary zexts and truncs.

LGTM

Nov 5 2018, 2:41 AM

Nov 1 2018

SjoerdMeijer accepted D53972: [ARM][CGP] Negative constant operand handling.

Cheers, LGTM!

Nov 1 2018, 7:14 AM
SjoerdMeijer added a comment to D53972: [ARM][CGP] Negative constant operand handling.

Had a first look, mostly nits.

Nov 1 2018, 2:48 AM

Oct 31 2018

SjoerdMeijer added a comment to D49671: [SchedModel] Propagate read advance cycles to implicit operands outside instruction descriptor.

Apologies for being late to the party, but I am now looking into this too because we've seen some significant regressions with this change committed.
I am not blaming this commit, not yet, because I haven't fully understood the problem yet. As I am new to this area, it wanted to dump some initial thoughts here (because it takes me some time to get up to speed), perhaps people can comment.

Oct 31 2018, 6:20 AM

Oct 29 2018

SjoerdMeijer updated the diff for D53710: [FileCheck] Warn if a prefix is only used in LABEL checks.
  • Instead of a Dk_Note, it is a DK_Warning now.
  • Added test cases; hopefully demonstrating that when we have a prefix list, and one of them is used, we don't issue the warning.
Oct 29 2018, 3:31 AM
SjoerdMeijer committed rL345491: [ARM][NFC] Fix test inlineasm-X-allocation.ll.
[ARM][NFC] Fix test inlineasm-X-allocation.ll
Oct 29 2018, 1:49 AM