- User Since
- Feb 5 2014, 1:36 AM (228 w, 1 d)
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.
Tue, Jun 5
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.
Apr 27 2018
Apr 26 2018
Apr 25 2018
Apr 11 2018
Apr 5 2018
Apr 3 2018
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.
Mar 6 2018
Mar 5 2018
Mar 1 2018
Feb 27 2018
Feb 21 2018
LGTM, but please wait a day to let other time-zones comment.
Feb 16 2018
Feb 14 2018
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?
Feb 13 2018
Feb 12 2018
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).
Feb 9 2018
Add a triple to the test.
Feb 8 2018
Was only wondering about the Cortex-A8 -> A15 change in the test run line.
Feb 7 2018
LGTM, but please wait a day to let other time zones comment.
Feb 6 2018
Would it be possible to enable this for all targets, not just ARM? Maybe this could be put into the constructor of LLVMTargetMachine?
Feb 5 2018
LGTM with a few nits (see inline comments), but please give 24 hours for other time zones to comment before committing.
Feb 2 2018
Feb 1 2018
Jan 31 2018
Thanks for making these changes, LGTM.
Jan 30 2018
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.
Jan 29 2018
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.
Jan 25 2018
Ok, I agree with the idea of committing this as a starting point and developing it gradually. Just a few nits left.
Jan 24 2018
Jan 23 2018
Jan 19 2018
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.
Jan 17 2018
I've spotted a missed optimisation in the tests, but that should be done as a separate patch.
Jan 16 2018
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.