Add switch instruction to Subzero bitcode reader.

src/PNaClTranslator.cpp

 //===- subzero/src/PNaClTranslator.cpp - ICE from bitcode -----------------===//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the PNaCl bitcode file to Ice, to machine code
@@ skipping 989 matching lines @@
     assert(Op);
     // Check if simple push works.
     uint32_t LocalIndex = Index - CachedNumGlobalValueIDs;
     if (LocalIndex == LocalOperands.size()) {
       LocalOperands.push_back(Op);
       return;
     }
 
     // Must be forward reference, expand vector to accommodate.
     if (LocalIndex >= LocalOperands.size())
-      LocalOperands.resize(LocalIndex+1);
+      LocalOperands.resize(LocalIndex + 1);
 
     // If element not defined, set it.
     Ice::Operand *OldOp = LocalOperands[LocalIndex];
     if (OldOp == NULL) {
       LocalOperands[LocalIndex] = Op;
       return;
     }
 
     // See if forward reference matches.
     if (OldOp == Op)
       return;
 
     // Error has occurred.
     std::string Buffer;
     raw_string_ostream StrBuf(Buffer);
-    StrBuf << "Multiple definitions for index " << Index
-           << ": " << *Op << " and " << *OldOp;
+    StrBuf << "Multiple definitions for index " << Index << ": " << *Op
+           << " and " << *OldOp;
     Error(StrBuf.str());
     // TODO(kschimpf) Remove error recovery once implementation complete.
     LocalOperands[LocalIndex] = Op;
   }
 
   // Returns the relative operand (wrt to BaseIndex) referenced by
   // the given value Index.
   Ice::Operand *getRelativeOperand(int32_t Index, int32_t BaseIndex) {
     return getOperand(convertRelativeToAbsIndex(Index, BaseIndex));
   }
 
   // Returns the absolute index of the next value generating instruction.
-  uint32_t getNextInstIndex() const {
-    return NextLocalInstIndex;
-  }
+  uint32_t getNextInstIndex() const { return NextLocalInstIndex; }
 
   // Generates type error message for binary operator Op
   // operating on Type OpTy.
   void ReportInvalidBinaryOp(Ice::InstArithmetic::OpKind Op, Ice::Type OpTy);
 
   // Validates if integer logical Op, for type OpTy, is valid.
   // Returns true if valid. Otherwise generates error message and
   // returns false.
   bool isValidIntegerLogicalOp(Ice::InstArithmetic::OpKind Op, Ice::Type OpTy) {
     if (Ice::isIntegerType(OpTy))
@@ skipping 622 matching lines @@
       Ice::CfgNode *ThenBlock = getBranchBasicBlock(Values[0]);
       Ice::CfgNode *ElseBlock = getBranchBasicBlock(Values[1]);
       if (ThenBlock == NULL || ElseBlock == NULL)
         return;
       CurrentNode->appendInst(
           Ice::InstBr::create(Func, Cond, ThenBlock, ElseBlock));
     }
     InstIsTerminating = true;
     break;
   }
+  case naclbitc::FUNC_CODE_INST_SWITCH: {
+    // SWITCH: [Condty, Cond, BbIndex, NumCases Case ...]
+    // where Case = [1, 1, Value, BbIndex].
+    //
+    // Note: Unlike most instructions, we don't infer the type of
+    // Cond, but provide it as a separate field. There are also
+    // unnecesary data fields (i.e. constants 1).  These were not
+    // cleaned up in PNaCl bitcode because the bitcode format was
+    // already frozen when the problem was noticed.
+    if (!isValidRecordSizeAtLeast(4, "function block switch"))
+      return;
+    Ice::Type CondTy =
+        Context->convertToIceType(Context->getTypeByID(Values[0]));

[Jim Stichnoth, 2014/09/17 20:18:20]

Consider defining "unsigned ValIndex = 0;" up here, then using Values[ValIndex]
here, and using Values[++ValIndex] where [1], [2], and [3] are used below.

(Comment continued on line 1721.)

[Karl, 2014/09/17 22:07:19]

To try and make things clearer, I renamed ValIndex to ValCaseIndex, since it
represents the starting location of a case in the list of values. I then made
all accessors into Values (for a case) be relative
to this value.

+    if (!Ice::isScalarIntegerType(CondTy) || CondTy == Ice::IceType_i64) {

[Jim Stichnoth, 2014/09/17 20:18:20]

You can have a switch on an i64 type.

[Karl, 2014/09/17 22:07:18]

Done.

+      std::string Buffer;
+      raw_string_ostream StrBuf(Buffer);
+      StrBuf << "Case condition must be non-wide integer. Found: " << CondTy;
+      Error(StrBuf.str());
+      return;
+    }
+    Ice::SizeT BitWidth = Ice::getScalarIntBitWidth(CondTy);
+    Ice::Operand *Cond = getRelativeOperand(Values[1], BaseIndex);
+    if (CondTy != Cond->getType()) {
+      std::string Buffer;
+      raw_string_ostream StrBuf(Buffer);
+      StrBuf << "Case condition expects type " << CondTy
+             << ". Found: " << Cond->getType();
+      Error(StrBuf.str());
+      return;
+    }
+    Ice::CfgNode *DefaultLabel = getBranchBasicBlock(Values[2]);
+    unsigned NumCases = Values[3];
+    if (!isValidRecordSize(4 + NumCases * 4, "Function block switch"))
+      return;
+    Ice::InstSwitch *Switch =
+        Ice::InstSwitch::create(Func, NumCases, Cond, DefaultLabel);
+    unsigned ValIndex = 4;
+    for (unsigned CaseIndex = 0; CaseIndex < NumCases; ++CaseIndex) {
+      if (Values[ValIndex] != 1) {

[Jim Stichnoth, 2014/09/17 20:18:20]

(continuing comment from line 1695)

ValIndex can then be changed to ++ValIndex here...

(comment continued on line 1741)

[Karl, 2014/09/17 22:07:19]

See comment above.

+        std::string Buffer;
+        raw_string_ostream StrBuf(Buffer);
+        StrBuf << "1 is expected for Values[" << ValIndex
+               << "] in switch record. Found: " << Values[ValIndex];
+        Error(StrBuf.str());
+        return;
+      }
+      if (Values[++ValIndex] != 1) {

[Jim Stichnoth, 2014/09/17 20:18:20]

Can this error test be combined with above?  E.g.

if (Values[ValIndex] != 1 || Values[++ValIndex] != 1) {

since the error message is the same.

[Karl, 2014/09/17 22:07:19]

Done.

+        std::string Buffer;
+        raw_string_ostream StrBuf(Buffer);
+        StrBuf << "1 is expected for Values[" << ValIndex
+               << "] in switch record. Found: " << Values[ValIndex];
+        Error(StrBuf.str());
+        return;
+      }
+      APInt Value(BitWidth, NaClDecodeSignRotatedValue(Values[++ValIndex]),
+                  true);
+      Ice::CfgNode *Label = getBranchBasicBlock(Values[++ValIndex]);
+      Switch->addBranch(CaseIndex, Value.getSExtValue(), Label);
+      ++ValIndex;

[Jim Stichnoth, 2014/09/17 20:18:20]

(continuing comment from line 1721)

and this ++ValIndex can be removed.

[Karl, 2014/09/17 22:07:19]

Removed.

+    }
+    CurrentNode->appendInst(Switch);
+    InstIsTerminating = true;
+    break;
+  }
   case naclbitc::FUNC_CODE_INST_UNREACHABLE: {
     // UNREACHABLE: []
     if (!isValidRecordSize(0, "function block unreachable"))
       return;
     CurrentNode->appendInst(
         Ice::InstUnreachable::create(Func));
     InstIsTerminating = true;
     break;
   }
   case naclbitc::FUNC_CODE_INST_PHI: {
@@ skipping 79 matching lines @@
     if (!isValidLoadStoreAlignment(Alignment, Value->getType(), "Store"))
       return;
     CurrentNode->appendInst(
         Ice::InstStore::create(Func, Value, Address, Alignment));
     break;
   }
   case naclbitc::FUNC_CODE_INST_FORWARDTYPEREF: {
     // FORWARDTYPEREF: [opval, ty]
     if (!isValidRecordSize(2, "function block forward type ref"))
       return;
-    setOperand(Values[0], createInstVar(
-        Context->convertToIceType(Context->getTypeByID(Values[1]))));
+    setOperand(Values[0], createInstVar(Context->convertToIceType(
+                              Context->getTypeByID(Values[1]))));
     break;
   }
-  case naclbitc::FUNC_CODE_INST_SWITCH:
   case naclbitc::FUNC_CODE_INST_CALL:
   case naclbitc::FUNC_CODE_INST_CALL_INDIRECT:
   default:
     // Generate error message!
     BlockParserBaseClass::ProcessRecord();
     break;
   }
 }
 
 /// Parses constants within a function block.
@@ skipping 389 matching lines @@
   if (TopLevelBlocks != 1) {
     errs() << IRFilename
            << ": Contains more than one module. Found: " << TopLevelBlocks
            << "\n";
     ErrorStatus = true;
   }
   return;
 }
 
 } // end of namespace Ice