[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