SjoerdMeijer (Sjoerd Meijer)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 26 2016, 2:17 AM (86 w, 3 d)

Recent Activity

Today

SjoerdMeijer added a comment to D35946: Fix printing of alias instructions by removing redundant spacing.

looks reasonable to me

Fri, Sep 22, 4:04 AM
SjoerdMeijer added a comment to D23601: [TII] add new target hook isAdd.

The context of this work was to recognise loop induction statements in Machine Instructions/Loops and to allow calculation of loop iteration counts. And isAdd provides more semantic information about a MI and helps recognising induction variables. Thus, this was the ground work for D27193, which was the next step to recognise loop induction. This patch was abandoned because the Quentin's comment was that the analysis was too ad-hoc and missing some formalism. The reason this is used only in the Hexagon backend, is because I refactored it and took it from the Hexagon backend. The Hexagon backend has a lot of MachineLoop/MI analysis and other goodness (sw pipeliner) that is mostly target independent and could be made available, but a few target specific hooks are required to do this.
Now, I really would like to pick up this work again because I still think it will be valuable, but need to rethink strategy after abandoning D27193. Probably that will be a some SVEC-like approach, but hopefully that can be a light-weight implementation to recognise simple loops (which is what we're doing anyway). This will probably be quite some work, so in the mean time, I really don't mind removing or moving isAdd back to the Hexagon backend. I could also dig up the exact requirements and document that if it's okay to keep it.

Fri, Sep 22, 12:40 AM

Mon, Sep 18

SjoerdMeijer committed rL313531: [ARM] Fix for indexed dot product instruction descriptions.
[ARM] Fix for indexed dot product instruction descriptions
Mon, Sep 18, 7:19 AM
SjoerdMeijer closed D37968: [ARM] Fix for indexed dot product instruction descriptions by committing rL313531: [ARM] Fix for indexed dot product instruction descriptions.
Mon, Sep 18, 7:19 AM
SjoerdMeijer added a comment to D37968: [ARM] Fix for indexed dot product instruction descriptions.

Thanks for checking!

Mon, Sep 18, 7:10 AM
SjoerdMeijer added a comment to D37968: [ARM] Fix for indexed dot product instruction descriptions.

I was just replying, but yes, there are only 4 bits available to encode Vm, the other M bit is the index.

Mon, Sep 18, 7:03 AM
SjoerdMeijer added a comment to D37968: [ARM] Fix for indexed dot product instruction descriptions.

Hi, the full architecture specification is publicly available here:

Mon, Sep 18, 5:03 AM
SjoerdMeijer created D37968: [ARM] Fix for indexed dot product instruction descriptions.
Mon, Sep 18, 3:54 AM

Fri, Sep 15

SjoerdMeijer committed rL313351: [AArch64] allow v8f16 types when FullFP16 is supported.
[AArch64] allow v8f16 types when FullFP16 is supported
Fri, Sep 15, 2:26 AM
SjoerdMeijer closed D37802: [AArch64] allow v8f16 types when FullFP16 is supported by committing rL313351: [AArch64] allow v8f16 types when FullFP16 is supported.
Fri, Sep 15, 2:26 AM
SjoerdMeijer accepted D37516: [ARM] Implement isTruncateFree.

Looks reasonable to me.

Fri, Sep 15, 1:34 AM

Wed, Sep 13

SjoerdMeijer committed rL313152: This adds the _Float16 preprocessor macro definitions..
This adds the _Float16 preprocessor macro definitions.
Wed, Sep 13, 8:24 AM
SjoerdMeijer closed D34695: _Float16 preprocessor macro definitions by committing rL313152: This adds the _Float16 preprocessor macro definitions..
Wed, Sep 13, 8:24 AM
SjoerdMeijer added a comment to D34695: _Float16 preprocessor macro definitions.

many thanks for reviewing and your help.

Wed, Sep 13, 8:20 AM
SjoerdMeijer created D37802: [AArch64] allow v8f16 types when FullFP16 is supported.
Wed, Sep 13, 6:06 AM
SjoerdMeijer updated the diff for D34695: _Float16 preprocessor macro definitions.

Fixed the typos, and added tests.

Wed, Sep 13, 1:42 AM

Tue, Sep 12

SjoerdMeijer added inline comments to D34695: _Float16 preprocessor macro definitions.
Tue, Sep 12, 9:05 AM
SjoerdMeijer committed rL313017: [AArch64] ISel: Add some debug messages to LowerBUILDVECTOR. NFC..
[AArch64] ISel: Add some debug messages to LowerBUILDVECTOR. NFC.
Tue, Sep 12, 3:25 AM
SjoerdMeijer closed D37676: [AArch64] ISel: Add some debug messages to LowerBUILDVECTOR. NFC. by committing rL313017: [AArch64] ISel: Add some debug messages to LowerBUILDVECTOR. NFC..
Tue, Sep 12, 3:25 AM

Mon, Sep 11

SjoerdMeijer created D37676: [AArch64] ISel: Add some debug messages to LowerBUILDVECTOR. NFC..
Mon, Sep 11, 1:30 AM

Fri, Sep 8

SjoerdMeijer committed rL312798: Add _Float16 as a C/C++ source language type.
Add _Float16 as a C/C++ source language type
Fri, Sep 8, 9:44 AM
SjoerdMeijer committed rL312794: Recommit "Add _Float16 as a C/C++ source language type".
Recommit "Add _Float16 as a C/C++ source language type"
Fri, Sep 8, 8:16 AM
SjoerdMeijer committed rL312784: Revert "Add _Float16 as a C/C++ source language type".
Revert "Add _Float16 as a C/C++ source language type"
Fri, Sep 8, 3:24 AM
SjoerdMeijer committed rL312781: Add _Float16 as a C/C++ source language type.
Add _Float16 as a C/C++ source language type
Fri, Sep 8, 2:44 AM
SjoerdMeijer closed D33719: Add _Float16 as a C/C++ source language type by committing rL312781: Add _Float16 as a C/C++ source language type.
Fri, Sep 8, 2:43 AM
SjoerdMeijer added a comment to D33719: Add _Float16 as a C/C++ source language type.

Many thanks for reviewing and your help!

Fri, Sep 8, 2:38 AM

Wed, Sep 6

SjoerdMeijer added a comment to D33719: Add _Float16 as a C/C++ source language type.

I am going to commit this within a few days. That looks reasonable to me given that the comments in the last reviews were very minor (which I have of course addressed already). Also, in case of issues, I am guessing fixes and/or addition can be easily done post-commit, and if not a revert is cheap.

Wed, Sep 6, 8:49 AM

Wed, Aug 30

SjoerdMeijer accepted D36792: [AArch64] v8.3-a complex number support.

Hi Sam, many thanks for refactoring this. Looks really good now! Just 2 nits inlined, but no need for another review.
Cheers, Sjoerd.

Wed, Aug 30, 7:44 AM
SjoerdMeijer added inline comments to D36792: [AArch64] v8.3-a complex number support.
Wed, Aug 30, 5:15 AM
SjoerdMeijer updated the diff for D33719: Add _Float16 as a C/C++ source language type.

Comments addressed. Thanks for reviewing.

Wed, Aug 30, 3:03 AM
SjoerdMeijer committed rL312104: [AArch64] allow v4f16 types when FullFP16 is supported.
[AArch64] allow v4f16 types when FullFP16 is supported
Wed, Aug 30, 1:39 AM
SjoerdMeijer closed D37145: [AArch64] allow v4f16 types when FullFP16 is supported by committing rL312104: [AArch64] allow v4f16 types when FullFP16 is supported.
Wed, Aug 30, 1:39 AM

Tue, Aug 29

SjoerdMeijer updated the diff for D33719: Add _Float16 as a C/C++ source language type.

No changes were needed to make the conversions work, the existing logic is taking care of that, but I agree it doesn't hurt to add a few test cases. So I've added tests to both files, and cleaned up that comment.

Tue, Aug 29, 8:57 AM
SjoerdMeijer updated the diff for D37145: [AArch64] allow v4f16 types when FullFP16 is supported.

Thanks for reviewing! I've addressed your comments: have set operation action to 'promote', added checks to test fcmp, and also added checks when fp16 is disabled.

Tue, Aug 29, 7:19 AM
SjoerdMeijer updated the diff for D37145: [AArch64] allow v4f16 types when FullFP16 is supported.

Fixed typo in test case type declaration.

Tue, Aug 29, 4:06 AM
SjoerdMeijer added a comment to D37145: [AArch64] allow v4f16 types when FullFP16 is supported.

Forgot to mention that I will address the v8f16 types in a separate patch.

Tue, Aug 29, 1:24 AM

Fri, Aug 25

SjoerdMeijer added inline comments to D36792: [AArch64] v8.3-a complex number support.
Fri, Aug 25, 7:34 AM
SjoerdMeijer created D37145: [AArch64] allow v4f16 types when FullFP16 is supported.
Fri, Aug 25, 6:51 AM

Thu, Aug 24

SjoerdMeijer committed rL311662: [AArch64] Add FMOVH0: materialize 0 using zero register for f16 values.
[AArch64] Add FMOVH0: materialize 0 using zero register for f16 values
Thu, Aug 24, 7:48 AM
SjoerdMeijer closed D37102: [AArch64] Add FMOVH0: materialize 0 using zero register for f16 values by committing rL311662: [AArch64] Add FMOVH0: materialize 0 using zero register for f16 values.
Thu, Aug 24, 7:48 AM
SjoerdMeijer accepted D37106: [Driver][AArch64] Tests for rdm feature..

Thanks for fixing this.

Thu, Aug 24, 7:22 AM
SjoerdMeijer accepted D37081: [TargetParser][AArch64] Add support for RDM feature in the target parser..

lgtm

Thu, Aug 24, 5:59 AM
SjoerdMeijer updated the diff for D37102: [AArch64] Add FMOVH0: materialize 0 using zero register for f16 values.

Added test case.

Thu, Aug 24, 4:15 AM
SjoerdMeijer created D37102: [AArch64] Add FMOVH0: materialize 0 using zero register for f16 values.
Thu, Aug 24, 3:32 AM
SjoerdMeijer committed rL311646: [AArch64] Custom lowering of copysign f16.
[AArch64] Custom lowering of copysign f16
Thu, Aug 24, 2:22 AM
SjoerdMeijer closed D36893: [AArch64] Custom lowering of copysign f16 by committing rL311646: [AArch64] Custom lowering of copysign f16.
Thu, Aug 24, 2:22 AM
SjoerdMeijer committed rL311635: [AArch64] fix for fcos and frem f16 promotion.
[AArch64] fix for fcos and frem f16 promotion
Thu, Aug 24, 12:45 AM
SjoerdMeijer closed D37071: [AArch64] fix for fcos and frem f16 promotion. by committing rL311635: [AArch64] fix for fcos and frem f16 promotion.
Thu, Aug 24, 12:45 AM
SjoerdMeijer added a comment to D37081: [TargetParser][AArch64] Add support for RDM feature in the target parser..

Looks good to me, was wondering if you're also planning to add a Clang patch to test +rdm?

Thu, Aug 24, 12:34 AM

Aug 23 2017

SjoerdMeijer created D37071: [AArch64] fix for fcos and frem f16 promotion..
Aug 23 2017, 9:19 AM
SjoerdMeijer added a comment to D36893: [AArch64] Custom lowering of copysign f16.

Just to clarify my last comment, I will repeat the exercise I did in D36396 allowing f16 scalars, and will now start working on allowing f16 vector types. I will then also add tests for these.

Aug 23 2017, 8:29 AM
SjoerdMeijer updated the diff for D36893: [AArch64] Custom lowering of copysign f16.

I have removed the special case, and added the vector types.
For the copysign intrinsics working on these vectors, I have not yet added new test cases, because it looks like I first have to first do some work to allow f16 vectors.

Aug 23 2017, 8:15 AM
SjoerdMeijer committed rL311533: [AArch64] ISel legalization debug messages. NFCI..
[AArch64] ISel legalization debug messages. NFCI.
Aug 23 2017, 1:19 AM
SjoerdMeijer closed D36964: [AArch64] ISel legalization debug messages. NFCI. by committing rL311533: [AArch64] ISel legalization debug messages. NFCI..
Aug 23 2017, 1:19 AM
SjoerdMeijer added a comment to D36964: [AArch64] ISel legalization debug messages. NFCI..

thanks for reviewing!

Aug 23 2017, 1:12 AM
SjoerdMeijer updated the diff for D36964: [AArch64] ISel legalization debug messages. NFCI..

Added missing spaces.

Aug 23 2017, 1:03 AM

Aug 22 2017

SjoerdMeijer updated the diff for D36964: [AArch64] ISel legalization debug messages. NFCI..

Hi Alex, thanks for the feedback. The difficulty here is finding the right balance of adding (good) debug messages, and not cluttering the code too much. I think what you are suggesting is what I had in my first revision: that had debug messages in the different AArch64 lowering functions. Then Sam's suggestion was to move some of these messages to SelectionDAG (which we did in D36984), which has 2 advantages: all targets benefit, and it does not clutter up the code. Also, I think this is quite difficult to get completely right the first time. So I think we will have to see what works and what is good and I will be adding more stuff as I go along. There's a lot of room for improvement here, but I already find these extra debug messages quite useful...

Aug 22 2017, 8:51 AM
SjoerdMeijer updated the diff for D36964: [AArch64] ISel legalization debug messages. NFCI..

Removed the debug messages that were moved to SelectionDAG in D36984

Aug 22 2017, 7:09 AM
SjoerdMeijer committed rL311444: [SelectionDAG] Add getNode debug messages.
[SelectionDAG] Add getNode debug messages
Aug 22 2017, 3:45 AM
SjoerdMeijer closed D36984: [SelectionDAG] Add getNode debug messages by committing rL311444: [SelectionDAG] Add getNode debug messages.
Aug 22 2017, 3:45 AM
SjoerdMeijer updated the diff for D36984: [SelectionDAG] Add getNode debug messages.

Hi Sam, thanks for reviewing. I have addressed your comments. Cheers.

Aug 22 2017, 2:45 AM
SjoerdMeijer committed rL311438: [AArch64] Cleanup of HasFullFP16 argument. NFC..
[AArch64] Cleanup of HasFullFP16 argument. NFC.
Aug 22 2017, 2:22 AM
SjoerdMeijer closed D36978: [AArch64] Cleanup of HasFullFP16 argument. NFC. by committing rL311438: [AArch64] Cleanup of HasFullFP16 argument. NFC..
Aug 22 2017, 2:22 AM
SjoerdMeijer updated the diff for D36984: [SelectionDAG] Add getNode debug messages.

Just added a few more debug messages.

Aug 22 2017, 1:10 AM

Aug 21 2017

SjoerdMeijer added a comment to D36964: [AArch64] ISel legalization debug messages. NFCI..

I will split this patch up in a target dependent, and target independent patch. I will continue here with the AArch64 specific messages, and I have created D36984 for some of the debug messages that I moved to SelectionDAG getNode functions.

Aug 21 2017, 3:36 PM
SjoerdMeijer created D36984: [SelectionDAG] Add getNode debug messages.
Aug 21 2017, 3:31 PM
SjoerdMeijer created D36978: [AArch64] Cleanup of HasFullFP16 argument. NFC..
Aug 21 2017, 1:03 PM
SjoerdMeijer added a comment to D36964: [AArch64] ISel legalization debug messages. NFCI..

The disadvantage of moving it to SelectionDAG is that we lose the context where the new nodes are created. In other words, the debug message will (potentially) become less specific. But since they currently do not have an awful lot of context, I think your suggestion makes sense. I will experiment to see if it works.

Aug 21 2017, 9:15 AM
SjoerdMeijer created D36964: [AArch64] ISel legalization debug messages. NFCI..
Aug 21 2017, 8:27 AM
SjoerdMeijer accepted D36666: [ObjC] Use consistent comment style in inline asm.

Looks reasonable to me.

Aug 21 2017, 2:41 AM

Aug 18 2017

SjoerdMeijer created D36893: [AArch64] Custom lowering of copysign f16.
Aug 18 2017, 1:30 PM
SjoerdMeijer accepted D36731: [ARM][AArch64] Cortex-A75 and Cortex-A55 tests.

Looks good to me too.

Aug 18 2017, 7:09 AM
SjoerdMeijer added a comment to D36731: [ARM][AArch64] Cortex-A75 and Cortex-A55 tests.

Ah, my message crossed with Renato's; I only noticed it after submitting mine (looks like we agree though:-))

Aug 18 2017, 4:31 AM
SjoerdMeijer added a comment to D36731: [ARM][AArch64] Cortex-A75 and Cortex-A55 tests.

"They also implement the RCpc AArch64 extension from ARMv8.3-A."

Aug 18 2017, 4:27 AM
SjoerdMeijer added reviewers for D36731: [ARM][AArch64] Cortex-A75 and Cortex-A55 tests: t.p.northover, rengolin.
Aug 18 2017, 4:01 AM
SjoerdMeijer added a reviewer for D36731: [ARM][AArch64] Cortex-A75 and Cortex-A55 tests: SjoerdMeijer.
Aug 18 2017, 4:00 AM
SjoerdMeijer committed rL311154: [AArch64] Do not promote f16 when subtarget HasFullFP16.
[AArch64] Do not promote f16 when subtarget HasFullFP16
Aug 18 2017, 3:54 AM
SjoerdMeijer closed D36396: [AArch64] Do not promote f16 when subtarget HasFullFP16 by committing rL311154: [AArch64] Do not promote f16 when subtarget HasFullFP16.
Aug 18 2017, 3:54 AM

Aug 15 2017

SjoerdMeijer updated the diff for D36396: [AArch64] Do not promote f16 when subtarget HasFullFP16.

I forgot to address some of Oliver's earlier comments:

  • I have added fixes for the conversion functions, and modified the tests,
  • I will address the codegen for copysign in a follow up patch, because it it requires a bit of custom lowering.
Aug 15 2017, 9:00 AM
SjoerdMeijer added a comment to D33719: Add _Float16 as a C/C++ source language type.

Ping

Aug 15 2017, 2:54 AM
SjoerdMeijer updated the diff for D36396: [AArch64] Do not promote f16 when subtarget HasFullFP16.

Hi Sam, Oliver, thanks for checking and reviewing. Your gut feelings were right: some more testing showed that target hook isFPImmLegal was not allowing f16. So I've modified that function, and added more tests.

Aug 15 2017, 2:47 AM

Aug 11 2017

SjoerdMeijer committed rL310708: [AArch64] Remove dotprod from base extension list.
[AArch64] Remove dotprod from base extension list
Aug 11 2017, 6:13 AM
SjoerdMeijer closed D36609: [AArch64] Remove dotprod from base extension list by committing rL310708: [AArch64] Remove dotprod from base extension list.
Aug 11 2017, 6:13 AM
SjoerdMeijer added a comment to D36609: [AArch64] Remove dotprod from base extension list.

thanks!

Aug 11 2017, 6:10 AM
SjoerdMeijer created D36609: [AArch64] Remove dotprod from base extension list.
Aug 11 2017, 5:46 AM
SjoerdMeijer committed rL310701: [ARM] Assembler support for the ARMv8.2a dot product instructions.
[ARM] Assembler support for the ARMv8.2a dot product instructions
Aug 11 2017, 2:53 AM
SjoerdMeijer closed D36575: [ARM] Assembler support for the ARMv8.2a dot product instructions by committing rL310701: [ARM] Assembler support for the ARMv8.2a dot product instructions.
Aug 11 2017, 2:53 AM

Aug 10 2017

SjoerdMeijer added a comment to D36575: [ARM] Assembler support for the ARMv8.2a dot product instructions.

Hi Sam,
Thanks for reviewing. I agree it looks odd, but the argument of not doing this, is that this is not user-facing. I.e., they are internal options and it's Clang's responsibilities to parse options and set the target features. Other optional instructions/extensions are also not predicated on e.g. v8.1a or v8.2a.

Aug 10 2017, 9:00 AM
SjoerdMeijer updated the diff for D36575: [ARM] Assembler support for the ARMv8.2a dot product instructions.

Added the disassembler tests.

Aug 10 2017, 7:01 AM
SjoerdMeijer added a comment to D36575: [ARM] Assembler support for the ARMv8.2a dot product instructions.

Oops, I now see that I forgot to add the disassmbler tests.

Aug 10 2017, 5:59 AM
SjoerdMeijer created D36575: [ARM] Assembler support for the ARMv8.2a dot product instructions.
Aug 10 2017, 5:56 AM
SjoerdMeijer accepted D36517: [AArch64] Enable ARMv8.3-A pointer authentication.

Looks okay to me.

Aug 10 2017, 1:38 AM

Aug 9 2017

SjoerdMeijer committed rL310480: [AArch64] Assembler support for the ARMv8.2a dot product instructions.
[AArch64] Assembler support for the ARMv8.2a dot product instructions
Aug 9 2017, 8:01 AM
SjoerdMeijer closed D36515: [AArch64] Assembler support for the ARMv8.2a dot product instructions by committing rL310480: [AArch64] Assembler support for the ARMv8.2a dot product instructions.
Aug 9 2017, 8:00 AM
SjoerdMeijer added a comment to D36515: [AArch64] Assembler support for the ARMv8.2a dot product instructions.

Thanks for the review!

Aug 9 2017, 8:00 AM
SjoerdMeijer accepted D36514: [ARM][AArch64] ARMv8.3-A enablement.

Looks reasonable to me.

Aug 9 2017, 6:48 AM
SjoerdMeijer created D36515: [AArch64] Assembler support for the ARMv8.2a dot product instructions.
Aug 9 2017, 5:54 AM

Aug 8 2017

SjoerdMeijer updated the summary of D36396: [AArch64] Do not promote f16 when subtarget HasFullFP16.
Aug 8 2017, 5:55 AM
SjoerdMeijer added a comment to D36396: [AArch64] Do not promote f16 when subtarget HasFullFP16.

Probably the subject/description of this patch is a bit confusing/misleading. I should have mentioned in the description that all the groundwork of adding the ARMv8.2-A FP16 (scalar) instructions was done D15014, which added all the instructions, and also e.g. an fpimm16 immediate type, and the required regression tests for these instructions. This patch is just a tweak to avoid promotions for FP16 instructions that support f16 operands.

Aug 8 2017, 5:51 AM

Aug 7 2017

SjoerdMeijer created D36396: [AArch64] Do not promote f16 when subtarget HasFullFP16.
Aug 7 2017, 7:51 AM