Fix problems with sandboxing and the ARM integrated assembler.

src/IceInstARM32.cpp

Index: src/IceInstARM32.cpp
diff --git a/src/IceInstARM32.cpp b/src/IceInstARM32.cpp
index 2590a10ed988fd08c81d3071166940c501128775..95e0e6f9c027a492bd3807ef452eb95c53905713 100644
--- a/src/IceInstARM32.cpp
+++ b/src/IceInstARM32.cpp
@@ -1263,12 +1263,6 @@ void InstARM32Pop::emitIAS(const Cfg *Func) const {
     // Note: Can only apply pop register if single register is not sp.
     assert((RegARM32::Encoded_Reg_sp != LastDest->getRegNum()) &&
            "Effects of pop register SP is undefined!");
-    // TODO(kschimpf) ARM sandbox does not allow the single register form of
-    // pop, and the popList form expects multiple registers. Convert this
-    // assert to a conditional check once it has been shown that popList
-    // works.
-    assert(!Func->getContext()->getFlags().getUseSandboxing() &&
-           "pop register not in ARM sandbox!");
     Asm->pop(LastDest, CondARM32::AL);
     break;
   default:
@@ -1369,12 +1363,6 @@ void InstARM32Push::emitIAS(const Cfg *Func) const {
     // Note: Can only apply push register if single register is not sp.
     assert((RegARM32::Encoded_Reg_sp != LastSrc->getRegNum()) &&
            "Effects of push register SP is undefined!");
-    // TODO(kschimpf) ARM sandbox does not allow the single register form of
-    // push, and the pushList form expects multiple registers. Convert this
-    // assert to a conditional check once it has been shown that pushList
-    // works.
-    assert(!Func->getContext()->getFlags().getUseSandboxing() &&
-           "push register not in ARM sandbox!");
     Asm->push(LastSrc, CondARM32::AL);
     break;
   }
@@ -1773,6 +1761,19 @@ void OperandARM32ShAmtImm::dump(const Cfg *, Ostream &Str) const {
   ShAmt->dump(Str);
 }
 
+OperandARM32FlexImm *OperandARM32FlexImm::create(Cfg *Func, Type Ty,
+                                                 uint32_t Imm,
+                                                 uint32_t RotateAmt) {
+  // The assembler wants the smallest rotation. Rotate if needed. Note: Imm is

[John, 2015/12/08 19:45:04]

Why is this needed?

[Karl, 2015/12/08 19:58:35]

If I don't do this, then values like rotate=2, Imm8=0xc will compile differently
by the two assemblers. I didn't discover this issue until I started compiling
with sandboxed examples (which appear to use the pair of constants quite often).

If the outputs are different, it is hard to compare an external assembler (when
using -filetype=asm) with the integrated assembler.

Hence, I added this to match what ARM states should be used if multiple
encodings are defined. I could have alternatively fixed all computations to not
do this, but it seemed harder.

+  // an 8-bit value.

[Jim Stichnoth, 2015/12/08 19:54:58]

Maybe validate Imm?  e.g.
  assert(Utils::IsUint(8, Imm));

[Karl, 2015/12/08 20:49:36]

Added assert.

+  while ((Imm & 0x03) == 0 && RotateAmt > 0) {
+    --RotateAmt;
+    Imm = Imm >> 2;
+  }
+  return new (Func->allocate<OperandARM32FlexImm>())
+      OperandARM32FlexImm(Func, Ty, Imm, RotateAmt);
+}
+
 void OperandARM32FlexImm::emit(const Cfg *Func) const {
   if (!BuildDefs::dump())
     return;