PNaCl wire format: Clean up representation of "alloca" instruction

For some reason, the size operand of "alloca" was represented using an
absolute value ID + type, unlike other instructions where relative
value IDs are used.

Change the "alloca" representation to be consistent with other
instructions, so that we can use PushValueAndType() in the writer and
getValue() in the reader.

Also take this opportunity to remove the field for alloca's result
type, since it's always i8* in PNaCl.

This is part of a cleanup to make forward reference handling stricter:
it removes a use of getOrCreateFnValueByID(), which isn't strict (that
is, it doesn't reject duplicate FORWARDTYPEREF records).

BUG=https://code.google.com/p/nativeclient/issues/detail?id=3507
TEST=PNaCl toolchain trybots

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

4 files changed, 40 insertions, 13 deletions

diff --git a/include/llvm/Bitcode/NaCl/NaClLLVMBitCodes.h b/include/llvm/Bitcode/NaCl/NaClLLVMBitCodes.h
index 0f3755abba..127f9c1e34 100644
--- a/include/llvm/Bitcode/NaCl/NaClLLVMBitCodes.h
+++ b/include/llvm/Bitcode/NaCl/NaClLLVMBitCodes.h
@@ -304,7 +304,7 @@ namespace naclbitc {
     FUNC_CODE_INST_PHI         = 16, // PHI:        [ty, val0,bb0, ...]
     // 17 is unused.
     // 18 is unused.
-    FUNC_CODE_INST_ALLOCA      = 19, // ALLOCA:     [instty, op, align]
+    FUNC_CODE_INST_ALLOCA      = 19, // ALLOCA:     [op, align]
     FUNC_CODE_INST_LOAD        = 20, // LOAD:       [op, align, vol]
     // 21 is unused.
     // 22 is unused.
diff --git a/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp b/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp
index 57dcea8953..8defdb5d1b 100644
--- a/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp
+++ b/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp
@@ -2283,16 +2283,15 @@ bool NaClBitcodeReader::ParseFunctionBody(Function *F) {
       break;
     }
 
-    case naclbitc::FUNC_CODE_INST_ALLOCA: { // ALLOCA: [instty, opty, op, align]
-      if (Record.size() != 4)
+    case naclbitc::FUNC_CODE_INST_ALLOCA: { // ALLOCA: [op, align]
+      if (Record.size() != 2)
+        return Error("Invalid ALLOCA record");
+      Value *Size;
+      unsigned OpNum = 0;
+      if (getValue(Record, OpNum, NextValueNo, Size))
         return Error("Invalid ALLOCA record");
-      PointerType *Ty =
-        dyn_cast_or_null<PointerType>(getTypeByID(Record[0]));
-      Type *OpTy = getTypeByID(Record[1]);
-      Value *Size = getOrCreateFnValueByID(Record[2], OpTy);
-      unsigned Align = Record[3];
-      if (!Ty || !Size) return Error("Invalid ALLOCA record");
-      I = new AllocaInst(Ty->getElementType(), Size, (1 << Align) >> 1);
+      unsigned Align = Record[1];
+      I = new AllocaInst(Type::getInt8Ty(Context), Size, (1 << Align) >> 1);
       InstructionList.push_back(I);
       break;
     }
diff --git a/lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp b/lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp
index b41b3c83c6..f25be94b3c 100644
--- a/lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp
+++ b/lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp
@@ -1358,10 +1358,10 @@ static void WriteInstruction(const Instruction &I, unsigned InstID,
   }
 
   case Instruction::Alloca:
+    if (!cast<AllocaInst>(&I)->getAllocatedType()->isIntegerTy(8))
+      report_fatal_error("Type of alloca instruction is not i8");
     Code = naclbitc::FUNC_CODE_INST_ALLOCA;
-    Vals.push_back(VE.getTypeID(I.getType()));
-    Vals.push_back(VE.getTypeID(I.getOperand(0)->getType()));
-    Vals.push_back(VE.getValueID(I.getOperand(0))); // size.
+    PushValueAndType(I.getOperand(0), InstID, Vals, VE, Stream); // size.
     Vals.push_back(Log2_32(cast<AllocaInst>(I).getAlignment())+1);
     break;
 
diff --git a/test/NaCl/Bitcode/alloca-operand.ll b/test/NaCl/Bitcode/alloca-operand.ll
new file mode 100644
index 0000000000..49df2fc8b0
--- /dev/null
+++ b/test/NaCl/Bitcode/alloca-operand.ll
@@ -0,0 +1,28 @@
+; RUN: llvm-as < %s | pnacl-freeze | pnacl-bcanalyzer -dump | FileCheck %s
+
+; Test that alloca's size operand is represented with a relative value
+; ID, the same as other instructions' operands.
+
+define external void @_start(i32 %arg) {
+; CHECK: <FUNCTION_BLOCK
+; CHECK: </CONSTANTS_BLOCK>
+
+  %size = mul i32 %arg, 4
+; CHECK-NEXT: <INST_BINOP
+  alloca i8, i32 %size
+; CHECK-NEXT: <INST_ALLOCA op0=1
+
+  ; Since the operand reference is a relative ID, references to %size
+  ; go up by 1 with each instruction.
+  alloca i8, i32 %size
+; CHECK-NEXT: <INST_ALLOCA op0=2
+  alloca i8, i32 %size
+; CHECK-NEXT: <INST_ALLOCA op0=3
+
+  ; Reference to a Constant operand.
+  alloca i8, i32 256
+; CHECK-NEXT: <INST_ALLOCA op0=5
+
+  ret void
+; CHECK-NEXT: <INST_RET
+}