[PATCH] [ARM64] Correct more bounds checks/diagnostics for arithmetic shift operands

Bradley Smith bradley.smith at arm.com
Mon May 12 02:10:04 PDT 2014


================
Comment at: lib/Target/ARM64/AsmParser/ARM64AsmParser.cpp:2476-2478
@@ -2468,3 +2475,5 @@
+  // diagnostics rather than ones different from out of range 32-bit shifts.
   if ((MCE->getValue() & 0x3f) != MCE->getValue()) {
-    Error(ExprLoc, "immediate value too large for shifter operand");
-    return MatchOperand_ParseFail;
+    Operands.push_back(ARM64Operand::CreateShifter(ARM64_AM::InvalidShift, 0, S,
+                                                   E, getContext()));
+  } else {
----------------
Tim Northover wrote:
> What goes wrong if we just add the shift and let the isXYZ methods take care of it? It seems like it might be more consistent way to handle things in general.
The shifts are encoded into a single integer in such a way that 0x3f is the maximum value for the shift, if you add one that exceeds this then it overflows into the shift type field in such a way that can turn it into a different valid shift type, if it doesn't assert first.

To be honest, I'm not sure why shifts/extracts/other things are encoded into a single integer like this, rather than using separate operand fields as AArch64 does, it seems to only cause headaches.

http://reviews.llvm.org/D3700






More information about the llvm-commits mailing list