- User Since
- Feb 5 2014, 1:36 AM (205 w, 6 d)
Fri, Jan 5
I approved this patch back in November.
Thu, Jan 4
This is described in the "Instruction endianness" section in the architecture reference manual, which is section A3.3.1 in the latest version of the v7-A/R manual. Here's the relevant bit:
Dec 13 2017
Dec 4 2017
This is very similar to what I'm doing with the multiple-near-miss diagnostics, trying to only emit messages which point out exactly what is wrong with an instruction, rather than simply the first thing the assembler found wrong. Hopefully this will be made obsolete once I get the near-miss mechanism completed and ported to non-ARM targets, but this looks like a reasonable point fix in the meantime. LGTM.
I've reverted this because it is causing a buildbot failure - http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/6552/steps/test-check-all/logs/stdio.
Dec 1 2017
Could you also add a test for the (i32, i64) case? We can currently emit the tail call for that (because r1 is not used).
Nov 29 2017
Nov 28 2017
Nov 27 2017
This patch doesn't have full context (git diff -U999999), please use that in future as it makes reviewing easier.
Nov 22 2017
Add messages to assertions.
Nov 21 2017
It looks like there are a lot of inaccuracies here still with regards to accepting r13/sp and r15/pc. Rejecting non-GPRs is obviously an improvement, but I don't think we should be adding diagnostics which give wrong information to the user.
Ping, the dependency (D39195) is now committed.
It's a shame that we have to fall back the the general "invalid operand for instruction" in all of these cases, but I don't see any easy fix for that, and it's obviously better than emitting the wrong diagnostic.
Nov 14 2017
Nov 13 2017
Nov 9 2017
Instead of only printing the operands that appear in the assembly string (some of which correspond to more then one MachineOperand), print a flattened list of operands. Kristof's example instruction now generates this documentation:
Nov 8 2017
Nov 7 2017
Oct 24 2017
I've double-checked, and the FLDMX and FSTMX are the only pre-UAL VFP instructions that don't have a UAL syntax, so I've removed all of the FLDMS anbd FLDMD aliases.
- Remove the -arm-asm-parser-dev-diags option
- Print the extra info straight to dbgs(), rather than appending to the diagnostic
Oct 23 2017
Isn't this going to be enabled by default some time in the near future?
Normal users will only ever see the "invalid operand for instruction" part of the diagnostic, regardess of whether they have assertions enabled or not. The extra info is only enabled when the -arm-arm-parser-dev-diags option is used (the "if (DevDiags)" check above), which isn't even exposed as a clang option (you'd need to pass it through with -mllvm).
It will probably take me a while to get the patch that uncovers this out, as it's quite complex (adding optional operands in the asm matcher, to remove some hacks like shouldOmitCCOutOperand in the asm parser and get more precise diagnostics), and I'm only working on this as a side project. Since this "bug" isn't observable at the moment I'm fine to leave this in review until the optional operands patch is published, so that it's clear why it is necessary. However, I think it should still be a separate patch, as it's not really related to the optional operands change, and would need to go in before it to avoid test failures.
+Saleem, who added these aliases in 2013.
Why do you think this should be kept downstream? It's a debug option which I've found useful while adding diagnostics for assembly options, none of which I plan on keeping downstream.
I'm not aware of any test case that can trigger this at the moment, because we get lucky with the ordering of opcodes in the assembly matcher table. However, other patches that I'm working on change that.
It's _not_ OK to select the Thumb version of this instruction in ARM mode, and that's what this patch is preventing. Normally, the T1I base class sets the Predicates list to [IsThumb], but the Requires class overrides the whole list rather than appending to it, so we need to explicitly include IsThumb in the list here. The ARM version of this instruction does have IsARM in the Predicates list, so doesn't get selected for Thumb targets.
Updated patch with more context.
Oct 13 2017
Oct 12 2017
Oct 11 2017
Committed as https://reviews.llvm.org/rL315445.
Oct 10 2017
Yes, I agree that it would be good to get this turned on for all targets. I'm developing it on ARM only at the moment so that I can make changes to the general mechanism without breaking lots of target's tests. Once it's stabilised, here's what I think needs doing to use it for other targets:
- Move the ReportNearMisses and FilterNearMisses from the ARM asm parser to somewhere target-independent. There is some ARM-specific code in there at the moment, so this will need some sort of callbacks into the target asm parser.
- Switch each asm parser to use the new mechanism. This should involve changing the signature of MatchInstruction, implementing any target-specific callbacks needed, and flipping the bit in the tablegen. I think the biggest part of this work will be updating the tests to match the new diagnostic format.
- Move MII at the base class
- Make MII a reference