[PATCH] starting from ARMv8, rGPR includes SP

Charlie Turner Charlie.Turner at arm.com
Mon Jun 22 11:14:40 PDT 2015


Hi Artyom, thanks for working on this!

Can you use Phabricator in the future, making sure to supply plenty of context? Most people prefer reviewing using that system :)

First patch,

+  case Match_RequiresV8:
+    return Error(IDLoc, "using SP in this instruction requires ARMv8 or later");

I see no test for this diagnostic in patch #1, but I do see a test in patch #2. Is there a good reason why these patches can't be reworked to test what the introduce in the same commit?

+  else if (isThumbOne()) {
+    // Some high-register supporting Thumb1 encodings only allow both registers
+    // to be from r0-r7 when in Thumb2.
+    if (Opc == ARM::tADDhirr && !hasV6MOps() &&
+        isARMLowRegister(Inst.getOperand(1).getReg()) &&
+        isARMLowRegister(Inst.getOperand(2).getReg()))
+      return Match_RequiresThumb2;
+    // Others only require ARMv6 or later.
+    else if (Opc == ARM::tMOVr && !hasV6Ops() &&
+             isARMLowRegister(Inst.getOperand(0).getReg()) &&
+             isARMLowRegister(Inst.getOperand(1).getReg()))
+      return Match_RequiresV6;
+  }

Please try and keep no functional changes like this separate.

As per John's comment, I agree that would be better, but I couldn't figure out how to do that when making the last change to BXJ.
I don't see a better way of doing this selection based on the target. The modelling of unpredictable instructions seems tricky with everything being statically generated as it is.

--Charlie.

-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2548782





More information about the llvm-commits mailing list