This restriction is in the Armv6M architecture, so I think this should be done in ARMv6m, so it also applies to cortex-m1 and sc000, and I think this also applies to ARMv8mBaseline.

It looks like we currently have two ways in which floating-point operands can get parsed, and this patch adds a third, which accepts values which overlap with both of the existing ones. Instead, this seems like a good time to tidy this up, always parsing to the same AArch64Operand, and using predicate functions to check if the value is valid for each operand type, as we do for integer operands with different ranges.

LGTM

LGTM, thanks.

This patch now changes 4 DAG nodes, but doesn't touch any tests for most of them. Are these changes NFC, and properly tested elsewhere? If not, this needs more tests.

LGTM, thanks.

LGTM.

LGTM, but please wait a day to let other time-zones comment.

Hi Simi,

LGTM

LGTM, thanks

The test case doesn't have to be exactly the -O0 output from clang, you should reduce it to clear IR which triggers the bug. To start with, are all of those allocas and memcpy calls required?

LGTM, thanks

LGTM, thanks

To have a better handling than this, we'd have to error out earlier saying M-class doesn't have NEON, which probably needs more knowledge than that piece of code has...

Geoff Berry suggested to do this in DAG Combine

Ping (also for the related D42478).

Add a triple to the test.

Was only wondering about the Cortex-A8 -> A15 change in the test run line.

LGTM, but please wait a day to let other time zones comment.

LGTM.

Thanks, LGTM.

Would it be possible to enable this for all targets, not just ARM? Maybe this could be put into the constructor of LLVMTargetMachine?

LGTM with a few nits (see inline comments), but please give 24 hours for other time zones to comment before committing.

Ping.

LGTM, thanks.

LGTM.

LGTM, thanks!

Thanks for making these changes, LGTM.

I'm still not convinced about the correctness of this transformation: you are turning code which contains truncates and extends into code that doesn't, without checking whether the top 16 bits could be relevant. This happens to be OK if the value is coming from/going to an fp16 arithmetic instruction, which ignores/clears the top 16 bits, but I don't think it's correct in all cases.

Is your concern that I am changing the semantics of these nodes because I am omitting this convert?

It might be intended to only apply to function arguments and returns, but those patterns for f16_to_fp and fp_to_f16 could match anywhere.

Have you tried adding tablegen patterns for bitconvert nodes between i16 and f16? That's how it currently works for f32<->i32, see the VMOVRS and VMOVSR instructions in ARMInstrVFP.td.

Ok, I agree with the idea of committing this as a starting point and developing it gradually. Just a few nits left.

Ping

Well, this patch doesn't change the actual conversion to MCInst, it just encodes more information into the conversion-function by using an indirection through the 'TiedAsmOperandTable'. It will still create the MCInst in the same way as it did before.

I was looking at your original version of this the other day to work out if it could be used to do 3-operand aliases of 2-operand Thumb1 instructions, and this (adding support for aliases) looks like it's what we would need for that.

I've spotted a missed optimisation in the tests, but that should be done as a separate patch.

This looks like a lot of additional complexity to deal with the case where we only have the conversion instructions, but you have made f16 a legal type.