Remove ARRAY/VECTOR types from PNaCl bitcode files.

The value selector list for switch statements are represented using
ARRAY/VECTOR constants, but this information is not put into the bitcode file.
This CL changes the value enumerator to not emit these constants.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=3649
R=mseaborn@chromium.org

Review URL: https://codereview.chromium.org/23653013

5 files changed, 103 insertions, 28 deletions

diff --git a/include/llvm/Bitcode/NaCl/NaClLLVMBitCodes.h b/include/llvm/Bitcode/NaCl/NaClLLVMBitCodes.h
index 59fadf26a2..32e3920b7d 100644
--- a/include/llvm/Bitcode/NaCl/NaClLLVMBitCodes.h
+++ b/include/llvm/Bitcode/NaCl/NaClLLVMBitCodes.h
@@ -91,8 +91,14 @@ namespace naclbitc {
 
     TYPE_CODE_HALF     =  10,   // Not used in PNaCl.
 
-    TYPE_CODE_ARRAY    = 11,    // ARRAY: [numelts, eltty]
-    TYPE_CODE_VECTOR   = 12,    // VECTOR: [numelts, eltty]
+    TYPE_CODE_ARRAY    = 11,    // PNaCl version 1 (early versions only)
+                                // ARRAY: [numelts, eltty]
+                                // Not used in PNaCl otherwise (i.e.
+                                // PNaCl versions 1+).
+    TYPE_CODE_VECTOR   = 12,    // PNaCl version 1 (early versions only)
+                                // VECTOR: [numelts, eltty]
+                                // Not used in PNaCl otherwise (i.e.
+                                // PNaCl versions 1+).
 
     // These are not with the other floating point types because they're
     // a late addition, and putting them in the right place breaks
diff --git a/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp b/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp
index bff4afb0c3..cccc5cee5a 100644
--- a/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp
+++ b/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp
@@ -508,6 +508,10 @@ bool NaClBitcodeReader::ParseTypeTableBody() {
       break;
     }
     case naclbitc::TYPE_CODE_ARRAY:     // ARRAY: [numelts, eltty]
+      // Deprecated. Only exists in early PNaCl version 1 bitcode files.
+      // TODO(kschimpf) Remove this as soon as is feasible.
+      if (GetPNaClVersion() >= 2)
+        return Error("Array types not supported in PNaCl bitcode");
       if (Record.size() < 2)
         return Error("Invalid ARRAY type record");
       if ((ResultTy = getTypeByID(Record[1])))
@@ -516,6 +520,10 @@ bool NaClBitcodeReader::ParseTypeTableBody() {
         return Error("Invalid ARRAY type element");
       break;
     case naclbitc::TYPE_CODE_VECTOR:    // VECTOR: [numelts, eltty]
+      // Deprecated. Only exists in early PNaCl version 1 bitcode files.
+      // TODO(kschimpf) Remove this as soon as is feasible.
+      if (GetPNaClVersion() >= 2)
+        return Error("Vector types not supported in PNaCl bitcode");
       if (Record.size() < 2)
         return Error("Invalid VECTOR type record");
       if ((ResultTy = getTypeByID(Record[1])))
diff --git a/lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp b/lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp
index 3d5d50cc20..ec13266221 100644
--- a/lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp
+++ b/lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp
@@ -253,23 +253,10 @@ static void WriteTypeTable(const NaClValueEnumerator &VE,
     }
     case Type::StructTyID:
       report_fatal_error("Struct types are not supported in PNaCl bitcode");
-    case Type::ArrayTyID: {
-      ArrayType *AT = cast<ArrayType>(T);
-      // ARRAY: [numelts, eltty]
-      Code = naclbitc::TYPE_CODE_ARRAY;
-      TypeVals.push_back(AT->getNumElements());
-      TypeVals.push_back(VE.getTypeID(AT->getElementType()));
-      AbbrevToUse = TYPE_ARRAY_ABBREV;
-      break;
-    }
-    case Type::VectorTyID: {
-      VectorType *VT = cast<VectorType>(T);
-      // VECTOR [numelts, eltty]
-      Code = naclbitc::TYPE_CODE_VECTOR;
-      TypeVals.push_back(VT->getNumElements());
-      TypeVals.push_back(VE.getTypeID(VT->getElementType()));
-      break;
-    }
+    case Type::ArrayTyID:
+      report_fatal_error("Array types are not supported in PNaCl bitcode");
+    case Type::VectorTyID:
+      report_fatal_error("Vector types are not supported in PNaCl bitcode");
     }
 
     // Emit the finished record.
diff --git a/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.cpp b/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.cpp
index be31779a3a..a58a421db2 100644
--- a/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.cpp
+++ b/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.cpp
@@ -94,9 +94,16 @@ NaClValueEnumerator::NaClValueEnumerator(const Module *M, uint32_t PNaClVersion)
         if (IsElidedCast(I))
           continue;
 
-        for (User::const_op_iterator OI = I->op_begin(), E = I->op_end();
-             OI != E; ++OI) {
-          EnumerateOperandType(*OI);
+        if (const SwitchInst *SI = dyn_cast<SwitchInst>(I)) {
+          // Handle switch instruction specially, so that we don't
+          // write out unnecessary vector/array types used to model case
+          // selectors.
+          EnumerateOperandType(SI->getCondition());
+        } else {
+          for (User::const_op_iterator OI = I->op_begin(), E = I->op_end();
+               OI != E; ++OI) {
+            EnumerateOperandType(*OI);
+          }
         }
         EnumerateType(I->getType());
       }
@@ -403,13 +410,22 @@ void NaClValueEnumerator::incorporateFunction(const Function &F) {
 
   // Add all function-level constants to the value table.
   for (Function::const_iterator BB = F.begin(), E = F.end(); BB != E; ++BB) {
-    for (BasicBlock::const_iterator I = BB->begin(), E = BB->end(); I!=E; ++I)
-      for (User::const_op_iterator OI = I->op_begin(), E = I->op_end();
-           OI != E; ++OI) {
-        if ((isa<Constant>(*OI) && !isa<GlobalValue>(*OI)) ||
-            isa<InlineAsm>(*OI))
-          EnumerateValue(*OI);
+    for (BasicBlock::const_iterator I = BB->begin(), E = BB->end(); I!=E; ++I) {
+      if (const SwitchInst *SI = dyn_cast<SwitchInst>(I)) {
+        // Handle switch instruction specially, so that we don't write
+        // out unnecessary vector/array constants used to model case selectors.
+        if (isa<Constant>(SI->getCondition())) {
+          EnumerateValue(SI->getCondition());
+        }
+      } else {
+        for (User::const_op_iterator OI = I->op_begin(), E = I->op_end();
+             OI != E; ++OI) {
+          if ((isa<Constant>(*OI) && !isa<GlobalValue>(*OI)) ||
+              isa<InlineAsm>(*OI))
+            EnumerateValue(*OI);
+        }
       }
+    }
     BasicBlocks.push_back(BB);
     ValueMap[BB] = BasicBlocks.size();
   }
diff --git a/test/NaCl/Bitcode/switch.ll b/test/NaCl/Bitcode/switch.ll
new file mode 100644
index 0000000000..9da6667bed
--- /dev/null
+++ b/test/NaCl/Bitcode/switch.ll
@@ -0,0 +1,58 @@
+; Test that we no longer put VECTOR/ARRAY type entries, associated with
+; switch instructions, into the bitcode file.
+
+; RUN: llvm-as < %s | pnacl-freeze --pnacl-version=1 \
+; RUN:              | pnacl-bcanalyzer -dump-records \
+; RUN:              | FileCheck %s
+
+; RUN: llvm-as < %s | pnacl-freeze --pnacl-version=2 \
+; RUN:              | pnacl-bcanalyzer -dump-records \
+; RUN:              | FileCheck %s
+
+; Test case where we switch on a variable.
+define void @SwitchVariable(i32) {
+  switch i32 %0, label %l1 [
+    i32 1, label %l2
+    i32 2, label %l2
+    i32 4, label %l3
+    i32 5, label %l3
+  ]
+  br label %end
+l1:
+  br label %end
+l2:
+  br label %end
+l3:
+  br label %end
+end:
+  ret void
+}
+
+; Test case where we switch on a constant.
+define void @SwitchConstant(i32) {
+  switch i32 3, label %l1 [
+    i32 1, label %l2
+    i32 2, label %l2
+    i32 4, label %l3
+    i32 5, label %l3
+  ]
+  br label %end
+l1:
+  br label %end
+l2:
+  br label %end
+l3:
+  br label %end
+end:
+  ret void
+}
+
+; CHECK:      <TYPE_BLOCK_ID>
+; CHECK-NEXT:   <NUMENTRY op0=5/>
+; CHECK-NEXT:   <VOID/>
+; CHECK-NEXT:   <LABEL/>
+; CHECK-NEXT:   <INTEGER op0=32/>
+; CHECK-NEXT:   <FUNCTION op0=0 op1=0 op2=2/>
+; CHECK-NEXT:   <POINTER op0=3 op1=0/>
+; CHECK-NEXT: </TYPE_BLOCK_ID>
+