- Finally nailed: test/Regression/Assembler/2002-08-16-ConstExprInlined.llx

    - ParseConstantPool was resolving reference to value using the function
      slot # instead of the global slot #.
  - Bytecode reader changes:
    - Remove the failure<> template from Bytecode Reader
    - Remove extraneous #includes
    - s/method/function/ a bit
    - Eliminate the fwdRefs class that just added abstraction where it was not
      needed, making things more complex.
    - Use a vector instead of a list for function signatures.


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@3366 91177308-0d34-0410-b5e6-96231b3b80d8

4 files changed, 292 insertions, 378 deletions

diff --git a/lib/Bytecode/Reader/ConstantReader.cpp b/lib/Bytecode/Reader/ConstantReader.cpp
index 37e201f280..88bfa3444a 100644
--- a/lib/Bytecode/Reader/ConstantReader.cpp
+++ b/lib/Bytecode/Reader/ConstantReader.cpp
@@ -10,17 +10,14 @@
 
 #include "ReaderInternals.h"
 #include "llvm/Module.h"
-#include "llvm/Constants.h"
-#include "llvm/GlobalVariable.h"
 #include <algorithm>
-#include <iostream>
 
 using std::make_pair;
 
 const Type *BytecodeParser::parseTypeConstant(const uchar *&Buf,
 					      const uchar *EndBuf) {
   unsigned PrimType;
-  if (read_vbr(Buf, EndBuf, PrimType)) return failure<const Type*>(0);
+  if (read_vbr(Buf, EndBuf, PrimType)) return 0;
 
   const Type *Val = 0;
   if ((Val = Type::getPrimitiveType((Type::PrimitiveID)PrimType)))
@@ -29,18 +26,18 @@ const Type *BytecodeParser::parseTypeConstant(const uchar *&Buf,
   switch (PrimType) {
   case Type::FunctionTyID: {
     unsigned Typ;
-    if (read_vbr(Buf, EndBuf, Typ)) return failure(Val);
+    if (read_vbr(Buf, EndBuf, Typ)) return Val;
     const Type *RetType = getType(Typ);
-    if (RetType == 0) return failure(Val);
+    if (RetType == 0) return Val;
 
     unsigned NumParams;
-    if (read_vbr(Buf, EndBuf, NumParams)) return failure(Val);
+    if (read_vbr(Buf, EndBuf, NumParams)) return Val;
 
     std::vector<const Type*> Params;
     while (NumParams--) {
-      if (read_vbr(Buf, EndBuf, Typ)) return failure(Val);
+      if (read_vbr(Buf, EndBuf, Typ)) return Val;
       const Type *Ty = getType(Typ);
-      if (Ty == 0) return failure(Val);
+      if (Ty == 0) return Val;
       Params.push_back(Ty);
     }
 
@@ -51,12 +48,12 @@ const Type *BytecodeParser::parseTypeConstant(const uchar *&Buf,
   }
   case Type::ArrayTyID: {
     unsigned ElTyp;
-    if (read_vbr(Buf, EndBuf, ElTyp)) return failure(Val);
+    if (read_vbr(Buf, EndBuf, ElTyp)) return Val;
     const Type *ElementType = getType(ElTyp);
-    if (ElementType == 0) return failure(Val);
+    if (ElementType == 0) return Val;
 
     unsigned NumElements;
-    if (read_vbr(Buf, EndBuf, NumElements)) return failure(Val);
+    if (read_vbr(Buf, EndBuf, NumElements)) return Val;
 
     BCR_TRACE(5, "Array Type Constant #" << ElTyp << " size=" 
               << NumElements << "\n");
@@ -66,23 +63,23 @@ const Type *BytecodeParser::parseTypeConstant(const uchar *&Buf,
     unsigned Typ;
     std::vector<const Type*> Elements;
 
-    if (read_vbr(Buf, EndBuf, Typ)) return failure(Val);
+    if (read_vbr(Buf, EndBuf, Typ)) return Val;
     while (Typ) {         // List is terminated by void/0 typeid
       const Type *Ty = getType(Typ);
-      if (Ty == 0) return failure(Val);
+      if (Ty == 0) return Val;
       Elements.push_back(Ty);
       
-      if (read_vbr(Buf, EndBuf, Typ)) return failure(Val);
+      if (read_vbr(Buf, EndBuf, Typ)) return Val;
     }
 
     return StructType::get(Elements);
   }
   case Type::PointerTyID: {
     unsigned ElTyp;
-    if (read_vbr(Buf, EndBuf, ElTyp)) return failure(Val);
+    if (read_vbr(Buf, EndBuf, ElTyp)) return Val;
     BCR_TRACE(5, "Pointer Type Constant #" << (ElTyp-14) << "\n");
     const Type *ElementType = getType(ElTyp);
-    if (ElementType == 0) return failure(Val);
+    if (ElementType == 0) return Val;
     return PointerType::get(ElementType);
   }
 
@@ -94,7 +91,7 @@ const Type *BytecodeParser::parseTypeConstant(const uchar *&Buf,
     std::cerr << __FILE__ << ":" << __LINE__
               << ": Don't know how to deserialize"
               << " primitive Type " << PrimType << "\n";
-    return failure(Val);
+    return Val;
   }
 }
 
@@ -149,7 +146,7 @@ bool BytecodeParser::parseTypeConstants(const uchar *&Buf, const uchar *EndBuf,
   //
   for (unsigned i = 0; i < NumEntries; ++i) {
     const Type *NewTy = parseTypeConstant(Buf, EndBuf), *OldTy = Tab[i].get();
-    if (NewTy == 0) return failure(true);
+    if (NewTy == 0) return true;
     BCR_TRACE(4, "#" << i << ": Read Type Constant: '" << NewTy <<
               "' Replacing: " << OldTy << "\n");
 
@@ -184,21 +181,21 @@ bool BytecodeParser::parseConstantValue(const uchar *&Buf, const uchar *EndBuf,
   // a ConstantExpr can be of any type, and has no explicit value.
   // 
   unsigned isExprNumArgs;               // 0 if not expr; numArgs if is expr
-  if (read_vbr(Buf, EndBuf, isExprNumArgs)) return failure(true);
+  if (read_vbr(Buf, EndBuf, isExprNumArgs)) return true;
   if (isExprNumArgs) {
     // FIXME: Encoding of constant exprs could be much more compact!
     unsigned Opcode;
     std::vector<Constant*> ArgVec;
     ArgVec.reserve(isExprNumArgs);
-    if (read_vbr(Buf, EndBuf, Opcode)) return failure(true);    
+    if (read_vbr(Buf, EndBuf, Opcode)) return true;    
 
     // Read the slot number and types of each of the arguments
     for (unsigned i = 0; i != isExprNumArgs; ++i) {
       unsigned ArgValSlot, ArgTypeSlot;
-      if (read_vbr(Buf, EndBuf, ArgValSlot)) return failure(true);
-      if (read_vbr(Buf, EndBuf, ArgTypeSlot)) return failure(true);
+      if (read_vbr(Buf, EndBuf, ArgValSlot)) return true;
+      if (read_vbr(Buf, EndBuf, ArgTypeSlot)) return true;
       const Type *ArgTy = getType(ArgTypeSlot);
-      if (ArgTy == 0) return failure(true);
+      if (ArgTy == 0) return true;
       
       BCR_TRACE(4, "CE Arg " << i << ": Type: '" << ArgTy << "'  slot: "
                 << ArgValSlot << "\n");
@@ -207,10 +204,23 @@ bool BytecodeParser::parseConstantValue(const uchar *&Buf, const uchar *EndBuf,
       Value *Val = getValue(ArgTy, ArgValSlot, false);
       Constant *C;
       if (Val) {
-        if (!(C = dyn_cast<Constant>(Val))) return failure(true);
+        if (!(C = dyn_cast<Constant>(Val))) return true;
         BCR_TRACE(5, "Constant Found in ValueTable!\n");
       } else {         // Nope... find or create a forward ref. for it
-        C = fwdRefs.GetFwdRefToConstant(ArgTy, ArgValSlot);
+        GlobalRefsType::iterator I = GlobalRefs.find(make_pair(Ty, ArgValSlot));
+  
+        if (I != GlobalRefs.end()) {
+          BCR_TRACE(5, "Previous forward ref found!\n");
+          C = cast<Constant>(I->second);
+        } else {
+          // Create a placeholder for the constant reference and
+          // keep track of the fact that we have a forward ref to recycle it
+          BCR_TRACE(5, "Creating new forward ref to a constant!\n");
+          C = new ConstPHolder(ArgTy, ArgValSlot);
+
+          // Keep track of the fact that we have a forward ref to recycle it
+          GlobalRefs.insert(make_pair(make_pair(ArgTy, ArgValSlot), C));
+        }
       }
       ArgVec.push_back(C);
     }
@@ -232,8 +242,8 @@ bool BytecodeParser::parseConstantValue(const uchar *&Buf, const uchar *EndBuf,
   switch (Ty->getPrimitiveID()) {
   case Type::BoolTyID: {
     unsigned Val;
-    if (read_vbr(Buf, EndBuf, Val)) return failure(true);
-    if (Val != 0 && Val != 1) return failure(true);
+    if (read_vbr(Buf, EndBuf, Val)) return true;
+    if (Val != 0 && Val != 1) return true;
     V = ConstantBool::get(Val == 1);
     break;
   }
@@ -242,15 +252,15 @@ bool BytecodeParser::parseConstantValue(const uchar *&Buf, const uchar *EndBuf,
   case Type::UShortTyID:
   case Type::UIntTyID: {
     unsigned Val;
-    if (read_vbr(Buf, EndBuf, Val)) return failure(true);
-    if (!ConstantUInt::isValueValidForType(Ty, Val)) return failure(true);
+    if (read_vbr(Buf, EndBuf, Val)) return true;
+    if (!ConstantUInt::isValueValidForType(Ty, Val)) return true;
     V = ConstantUInt::get(Ty, Val);
     break;
   }
 
   case Type::ULongTyID: {
     uint64_t Val;
-    if (read_vbr(Buf, EndBuf, Val)) return failure(true);
+    if (read_vbr(Buf, EndBuf, Val)) return true;
     V = ConstantUInt::get(Ty, Val);
     break;
   }
@@ -259,29 +269,29 @@ bool BytecodeParser::parseConstantValue(const uchar *&Buf, const uchar *EndBuf,
   case Type::ShortTyID:
   case Type::IntTyID: {
     int Val;
-    if (read_vbr(Buf, EndBuf, Val)) return failure(true);
-    if (!ConstantSInt::isValueValidForType(Ty, Val)) return failure(true);
+    if (read_vbr(Buf, EndBuf, Val)) return true;
+    if (!ConstantSInt::isValueValidForType(Ty, Val)) return true;
     V = ConstantSInt::get(Ty, Val);
     break;
   }
 
   case Type::LongTyID: {
     int64_t Val;
-    if (read_vbr(Buf, EndBuf, Val)) return failure(true);
+    if (read_vbr(Buf, EndBuf, Val)) return true;
     V = ConstantSInt::get(Ty, Val);
     break;
   }
 
   case Type::FloatTyID: {
     float F;
-    if (input_data(Buf, EndBuf, &F, &F+1)) return failure(true);
+    if (input_data(Buf, EndBuf, &F, &F+1)) return true;
     V = ConstantFP::get(Ty, F);
     break;
   }
 
   case Type::DoubleTyID: {
     double Val;
-    if (input_data(Buf, EndBuf, &Val, &Val+1)) return failure(true);
+    if (input_data(Buf, EndBuf, &Val, &Val+1)) return true;
     V = ConstantFP::get(Ty, Val);
     break;
   }
@@ -297,9 +307,9 @@ bool BytecodeParser::parseConstantValue(const uchar *&Buf, const uchar *EndBuf,
     std::vector<Constant*> Elements;
     while (NumElements--) {   // Read all of the elements of the constant.
       unsigned Slot;
-      if (read_vbr(Buf, EndBuf, Slot)) return failure(true);
+      if (read_vbr(Buf, EndBuf, Slot)) return true;
       Value *V = getValue(AT->getElementType(), Slot, false);
-      if (!V || !isa<Constant>(V)) return failure(true);
+      if (!V || !isa<Constant>(V)) return true;
       Elements.push_back(cast<Constant>(V));
     }
     V = ConstantArray::get(AT, Elements);
@@ -313,10 +323,10 @@ bool BytecodeParser::parseConstantValue(const uchar *&Buf, const uchar *EndBuf,
     std::vector<Constant *> Elements;
     for (unsigned i = 0; i < ET.size(); ++i) {
       unsigned Slot;
-      if (read_vbr(Buf, EndBuf, Slot)) return failure(true);
+      if (read_vbr(Buf, EndBuf, Slot)) return true;
       Value *V = getValue(ET[i], Slot, false);
       if (!V || !isa<Constant>(V))
-	return failure(true);
+	return true;
       Elements.push_back(cast<Constant>(V));      
     }
 
@@ -327,7 +337,7 @@ bool BytecodeParser::parseConstantValue(const uchar *&Buf, const uchar *EndBuf,
   case Type::PointerTyID: {
     const PointerType *PT = cast<const PointerType>(Ty);
     unsigned SubClass;
-    if (read_vbr(Buf, EndBuf, SubClass)) return failure(true);
+    if (read_vbr(Buf, EndBuf, SubClass)) return true;
     switch (SubClass) {
     case 0:    // ConstantPointerNull value...
       V = ConstantPointerNull::get(PT);
@@ -335,17 +345,35 @@ bool BytecodeParser::parseConstantValue(const uchar *&Buf, const uchar *EndBuf,
 
     case 1: {  // ConstantPointerRef value...
       unsigned Slot;
-      if (read_vbr(Buf, EndBuf, Slot)) return failure(true);
+      if (read_vbr(Buf, EndBuf, Slot)) return true;
       BCR_TRACE(4, "CPR: Type: '" << Ty << "'  slot: " << Slot << "\n");
 
-      // Check to see if we have already read this global variable yet...
+      // Check to see if we have already read this global variable...
       Value *Val = getValue(PT, Slot, false);
-      GlobalValue* GV;
+      GlobalValue *GV;
       if (Val) {
-        if (!(GV = dyn_cast<GlobalValue>(Val))) return failure(true);
+        if (!(GV = dyn_cast<GlobalValue>(Val))) return true;
         BCR_TRACE(5, "Value Found in ValueTable!\n");
       } else {         // Nope... find or create a forward ref. for it
-        GV = fwdRefs.GetFwdRefToGlobal(PT, Slot);
+        GlobalRefsType::iterator I = GlobalRefs.find(make_pair(PT, Slot));
+
+        if (I != GlobalRefs.end()) {
+          BCR_TRACE(5, "Previous forward ref found!\n");
+          GV = cast<GlobalValue>(I->second);
+        } else {
+          BCR_TRACE(5, "Creating new forward ref to a global variable!\n");
+
+	  // Create a placeholder for the global variable reference...
+          GlobalVariable *GVar =
+            new GlobalVariable(PT->getElementType(), false, true);
+          
+	  // Keep track of the fact that we have a forward ref to recycle it
+          GlobalRefs.insert(make_pair(make_pair(PT, Slot), GVar));
+          
+          // Must temporarily push this value into the module table...
+          TheModule->getGlobalList().push_back(GVar);
+          GV = GVar;
+        }
       }
       V = ConstantPointerRef::get(GV);
       break;
@@ -353,7 +381,7 @@ bool BytecodeParser::parseConstantValue(const uchar *&Buf, const uchar *EndBuf,
     
     default:
       BCR_TRACE(5, "UNKNOWN Pointer Constant Type!\n");
-      return failure(true);
+      return true;
     }
     break;
   }
@@ -362,7 +390,7 @@ bool BytecodeParser::parseConstantValue(const uchar *&Buf, const uchar *EndBuf,
     std::cerr << __FILE__ << ":" << __LINE__ 
               << ": Don't know how to deserialize constant value of type '"
               << Ty->getName() << "'\n";
-    return failure(true);
+    return true;
   }
 
   return false;
@@ -375,9 +403,9 @@ bool BytecodeParser::ParseConstantPool(const uchar *&Buf, const uchar *EndBuf,
     unsigned NumEntries, Typ;
 
     if (read_vbr(Buf, EndBuf, NumEntries) ||
-        read_vbr(Buf, EndBuf, Typ)) return failure(true);
+        read_vbr(Buf, EndBuf, Typ)) return true;
     const Type *Ty = getType(Typ);
-    if (Ty == 0) return failure(true);
+    if (Ty == 0) return true;
     BCR_TRACE(3, "Type: '" << Ty << "'  NumEntries: " << NumEntries << "\n");
 
     if (Typ == Type::TypeTyID) {
@@ -386,15 +414,22 @@ bool BytecodeParser::ParseConstantPool(const uchar *&Buf, const uchar *EndBuf,
       for (unsigned i = 0; i < NumEntries; ++i) {
 	Constant *I;
         int Slot;
-	if (parseConstantValue(Buf, EndBuf, Ty, I)) return failure(true);
-        assert(I && "parseConstantValue returned `!failure' and NULL result");
+	if (parseConstantValue(Buf, EndBuf, Ty, I)) return true;
+        assert(I && "parseConstantValue returned NULL!");
 	BCR_TRACE(4, "Read Constant: '" << I << "'\n");
-	if ((Slot = insertValue(I, Tab)) < 0) return failure(true);
-        resolveRefsToConstant(I, (unsigned) Slot);
+	if ((Slot = insertValue(I, Tab)) < 0) return true;
+
+        // If we are reading a function constant table, make sure that we adjust
+        // the slot number to be the real global constant number.
+        //
+        if (&Tab != &ModuleValues)
+          Slot += ModuleValues[Typ].size();
+
+        ResolveReferencesToValue(I, (unsigned)Slot);
       }
     }
   }
   
-  if (Buf > EndBuf) return failure(true);
+  if (Buf > EndBuf) return true;
   return false;
 }
diff --git a/lib/Bytecode/Reader/InstructionReader.cpp b/lib/Bytecode/Reader/InstructionReader.cpp
index 0de8f39c57..979b3944bc 100644
--- a/lib/Bytecode/Reader/InstructionReader.cpp
+++ b/lib/Bytecode/Reader/InstructionReader.cpp
@@ -16,14 +16,13 @@
 #include "llvm/iMemory.h"
 #include "llvm/iPHINode.h"
 #include "llvm/iOther.h"
-#include <iostream>
 using std::vector;
 using std::cerr;
 
 bool BytecodeParser::ParseRawInst(const uchar *&Buf, const uchar *EndBuf, 
 				  RawInst &Result) {
   unsigned Op, Typ;
-  if (read(Buf, EndBuf, Op)) return failure(true);
+  if (read(Buf, EndBuf, Op)) return true;
 
   // bits   Instruction format:        Common to all formats
   // --------------------------
@@ -70,40 +69,40 @@ bool BytecodeParser::ParseRawInst(const uchar *&Buf, const uchar *EndBuf,
     break;
   case 0:
     Buf -= 4;  // Hrm, try this again...
-    if (read_vbr(Buf, EndBuf, Result.Opcode)) return failure(true);
+    if (read_vbr(Buf, EndBuf, Result.Opcode)) return true;
     Result.Opcode >>= 2;
-    if (read_vbr(Buf, EndBuf, Typ)) return failure(true);
+    if (read_vbr(Buf, EndBuf, Typ)) return true;
     Result.Ty = getType(Typ);
-    if (Result.Ty == 0) return failure(true);
-    if (read_vbr(Buf, EndBuf, Result.NumOperands)) return failure(true);
+    if (Result.Ty == 0) return true;
+    if (read_vbr(Buf, EndBuf, Result.NumOperands)) return true;
 
     switch (Result.NumOperands) {
     case 0: 
       cerr << "Zero Arg instr found!\n"; 
-      return failure(true);  // This encoding is invalid!
+      return true;  // This encoding is invalid!
     case 1: 
-      if (read_vbr(Buf, EndBuf, Result.Arg1)) return failure(true);
+      if (read_vbr(Buf, EndBuf, Result.Arg1)) return true;
       break;
     case 2:
       if (read_vbr(Buf, EndBuf, Result.Arg1) || 
-	  read_vbr(Buf, EndBuf, Result.Arg2)) return failure(true);
+	  read_vbr(Buf, EndBuf, Result.Arg2)) return true;
       break;
     case 3:
       if (read_vbr(Buf, EndBuf, Result.Arg1) || 
 	  read_vbr(Buf, EndBuf, Result.Arg2) ||
-          read_vbr(Buf, EndBuf, Result.Arg3)) return failure(true);
+          read_vbr(Buf, EndBuf, Result.Arg3)) return true;
       break;
     default:
       if (read_vbr(Buf, EndBuf, Result.Arg1) || 
-	  read_vbr(Buf, EndBuf, Result.Arg2)) return failure(true);
+	  read_vbr(Buf, EndBuf, Result.Arg2)) return true;
 
       // Allocate a vector to hold arguments 3, 4, 5, 6 ...
       Result.VarArgs = new vector<unsigned>(Result.NumOperands-2);
       for (unsigned a = 0; a < Result.NumOperands-2; a++)
-	if (read_vbr(Buf, EndBuf, (*Result.VarArgs)[a])) return failure(true);
+	if (read_vbr(Buf, EndBuf, (*Result.VarArgs)[a])) return true;
       break;
     }
-    if (align32(Buf, EndBuf)) return failure(true);
+    if (align32(Buf, EndBuf)) return true;
     break;
   }
 
@@ -120,7 +119,7 @@ bool BytecodeParser::ParseInstruction(const uchar *&Buf, const uchar *EndBuf,
 				      Instruction *&Res) {
   RawInst Raw;
   if (ParseRawInst(Buf, EndBuf, Raw))
-    return failure(true);
+    return true;
 
   if (Raw.Opcode >= Instruction::FirstBinaryOp &&
       Raw.Opcode <  Instruction::NumBinaryOps  && Raw.NumOperands == 2) {
@@ -146,7 +145,7 @@ bool BytecodeParser::ParseInstruction(const uchar *&Buf, const uchar *EndBuf,
     case 1: 
     case 3: cerr << "Invalid phi node encountered!\n"; 
             delete PN; 
-	    return failure(true);
+	    return true;
     case 2: PN->addIncoming(getValue(Raw.Ty, Raw.Arg1),
 			    cast<BasicBlock>(getValue(Type::LabelTy,Raw.Arg2)));
       break;
@@ -156,7 +155,7 @@ bool BytecodeParser::ParseInstruction(const uchar *&Buf, const uchar *EndBuf,
       if (Raw.VarArgs->size() & 1) {
 	cerr << "PHI Node with ODD number of arguments!\n";
 	delete PN;
-	return failure(true);
+	return true;
       } else {
         vector<unsigned> &args = *Raw.VarArgs;
         for (unsigned i = 0; i < args.size(); i+=2)
@@ -206,7 +205,7 @@ bool BytecodeParser::ParseInstruction(const uchar *&Buf, const uchar *EndBuf,
     if (Raw.NumOperands == 3 || Raw.VarArgs->size() & 1) {
       cerr << "Switch statement with odd number of arguments!\n";
       delete I;
-      return failure(true);
+      return true;
     }      
     
     vector<unsigned> &args = *Raw.VarArgs;
@@ -220,13 +219,13 @@ bool BytecodeParser::ParseInstruction(const uchar *&Buf, const uchar *EndBuf,
 
   case Instruction::Call: {
     Value *M = getValue(Raw.Ty, Raw.Arg1);
-    if (M == 0) return failure(true);
+    if (M == 0) return true;
 
     // Check to make sure we have a pointer to method type
     const PointerType *PTy = dyn_cast<PointerType>(M->getType());
-    if (PTy == 0) return failure(true);
+    if (PTy == 0) return true;
     const FunctionType *MTy = dyn_cast<FunctionType>(PTy->getElementType());
-    if (MTy == 0) return failure(true);
+    if (MTy == 0) return true;
 
     vector<Value *> Params;
     const FunctionType::ParamTypes &PL = MTy->getParamTypes();
@@ -236,39 +235,39 @@ bool BytecodeParser::ParseInstruction(const uchar *&Buf, const uchar *EndBuf,
 
       switch (Raw.NumOperands) {
       case 0: cerr << "Invalid call instruction encountered!\n";
-	return failure(true);
+	return true;
       case 1: break;
       case 2: Params.push_back(getValue(*It++, Raw.Arg2)); break;
       case 3: Params.push_back(getValue(*It++, Raw.Arg2)); 
-	if (It == PL.end()) return failure(true);
+	if (It == PL.end()) return true;
 	Params.push_back(getValue(*It++, Raw.Arg3)); break;
       default:
 	Params.push_back(getValue(*It++, Raw.Arg2));
 	{
 	  vector<unsigned> &args = *Raw.VarArgs;
 	  for (unsigned i = 0; i < args.size(); i++) {
-	    if (It == PL.end()) return failure(true);
+	    if (It == PL.end()) return true;
 	    // TODO: Check getValue for null!
 	    Params.push_back(getValue(*It++, args[i]));
 	  }
 	}
 	delete Raw.VarArgs;
       }
-      if (It != PL.end()) return failure(true);
+      if (It != PL.end()) return true;
     } else {
       if (Raw.NumOperands > 2) {
 	vector<unsigned> &args = *Raw.VarArgs;
-	if (args.size() < 1) return failure(true);
+	if (args.size() < 1) return true;
 
 	if ((args.size() & 1) != 0)
-	  return failure(true);  // Must be pairs of type/value
+	  return true;  // Must be pairs of type/value
 	for (unsigned i = 0; i < args.size(); i+=2) {
 	  const Type *Ty = getType(args[i]);
 	  if (Ty == 0)
-	    return failure(true);
+	    return true;
 	  
 	  Value *V = getValue(Ty, args[i+1]);
-	  if (V == 0) return failure(true);
+	  if (V == 0) return true;
 	  Params.push_back(V);
 	}
 	delete Raw.VarArgs;
@@ -280,13 +279,13 @@ bool BytecodeParser::ParseInstruction(const uchar *&Buf, const uchar *EndBuf,
   }
   case Instruction::Invoke: {
     Value *M = getValue(Raw.Ty, Raw.Arg1);
-    if (M == 0) return failure(true);
+    if (M == 0) return true;
 
     // Check to make sure we have a pointer to method type
     const PointerType *PTy = dyn_cast<PointerType>(M->getType());
-    if (PTy == 0) return failure(true);
+    if (PTy == 0) return true;
     const FunctionType *MTy = dyn_cast<FunctionType>(PTy->getElementType());
-    if (MTy == 0) return failure(true);
+    if (MTy == 0) return true;
 
     vector<Value *> Params;
     const FunctionType::ParamTypes &PL = MTy->getParamTypes();
@@ -295,27 +294,27 @@ bool BytecodeParser::ParseInstruction(const uchar *&Buf, const uchar *EndBuf,
     BasicBlock *Normal, *Except;
 
     if (!MTy->isVarArg()) {
-      if (Raw.NumOperands < 3) return failure(true);
+      if (Raw.NumOperands < 3) return true;
 
       Normal = cast<BasicBlock>(getValue(Type::LabelTy, Raw.Arg2));
       Except = cast<BasicBlock>(getValue(Type::LabelTy, args[0]));
 
       FunctionType::ParamTypes::const_iterator It = PL.begin();
       for (unsigned i = 1; i < args.size(); i++) {
-	if (It == PL.end()) return failure(true);
+	if (It == PL.end()) return true;
 	// TODO: Check getValue for null!
 	Params.push_back(getValue(*It++, args[i]));
       }
 
-      if (It != PL.end()) return failure(true);
+      if (It != PL.end()) return true;
     } else {
-      if (args.size() < 4) return failure(true);
+      if (args.size() < 4) return true;
 
       Normal = cast<BasicBlock>(getValue(Type::LabelTy, args[0]));
       Except = cast<BasicBlock>(getValue(Type::LabelTy, args[2]));
 
       if ((args.size() & 1) != 0)
-	return failure(true);  // Must be pairs of type/value
+	return true;  // Must be pairs of type/value
       for (unsigned i = 4; i < args.size(); i+=2) {
 	// TODO: Check getValue for null!
 	Params.push_back(getValue(getType(args[i]), args[i+1]));
@@ -327,62 +326,62 @@ bool BytecodeParser::ParseInstruction(const uchar *&Buf, const uchar *EndBuf,
     return false;
   }
   case Instruction::Malloc:
-    if (Raw.NumOperands > 2) return failure(true);
+    if (Raw.NumOperands > 2) return true;
     V = Raw.NumOperands ? getValue(Type::UIntTy, Raw.Arg1) : 0;
     Res = new MallocInst(Raw.Ty, V);
     return false;
 
   case Instruction::Alloca:
-    if (Raw.NumOperands > 2) return failure(true);
+    if (Raw.NumOperands > 2) return true;
     V = Raw.NumOperands ? getValue(Type::UIntTy, Raw.Arg1) : 0;
     Res = new AllocaInst(Raw.Ty, V);
     return false;
 
   case Instruction::Free:
     V = getValue(Raw.Ty, Raw.Arg1);
-    if (!isa<PointerType>(V->getType())) return failure(true);
+    if (!isa<PointerType>(V->getType())) return true;
     Res = new FreeInst(V);
     return false;
 
   case Instruction::Load:
   case Instruction::GetElementPtr: {
     vector<Value*> Idx;
-    if (!isa<PointerType>(Raw.Ty)) return failure(true);
+    if (!isa<PointerType>(Raw.Ty)) return true;
     const CompositeType *TopTy = dyn_cast<CompositeType>(Raw.Ty);
 
     switch (Raw.NumOperands) {
-    case 0: cerr << "Invalid load encountered!\n"; return failure(true);
+    case 0: cerr << "Invalid load encountered!\n"; return true;
     case 1: break;
     case 2:
-      if (!TopTy) return failure(true);
+      if (!TopTy) return true;
       Idx.push_back(V = getValue(TopTy->getIndexType(), Raw.Arg2));
-      if (!V) return failure(true);
+      if (!V) return true;
       break;
     case 3: {
-      if (!TopTy) return failure(true);
+      if (!TopTy) return true;
       Idx.push_back(V = getValue(TopTy->getIndexType(), Raw.Arg2));
-      if (!V) return failure(true);
+      if (!V) return true;
 
       const Type *ETy = MemAccessInst::getIndexedType(TopTy, Idx, true);
       const CompositeType *ElTy = dyn_cast_or_null<CompositeType>(ETy);
-      if (!ElTy) return failure(true);
+      if (!ElTy) return true;
 
       Idx.push_back(V = getValue(ElTy->getIndexType(), Raw.Arg3));
-      if (!V) return failure(true);
+      if (!V) return true;
       break;
     }
     default:
-      if (!TopTy) return failure(true);
+      if (!TopTy) return true;
       Idx.push_back(V = getValue(TopTy->getIndexType(), Raw.Arg2));
-      if (!V) return failure(true);
+      if (!V) return true;
 
       vector<unsigned> &args = *Raw.VarArgs;
       for (unsigned i = 0, E = args.size(); i != E; ++i) {
         const Type *ETy = MemAccessInst::getIndexedType(Raw.Ty, Idx, true);
         const CompositeType *ElTy = dyn_cast_or_null<CompositeType>(ETy);
-        if (!ElTy) return failure(true);
+        if (!ElTy) return true;
 	Idx.push_back(V = getValue(ElTy->getIndexType(), args[i]));
-	if (!V) return failure(true);
+	if (!V) return true;
       }
       delete Raw.VarArgs; 
       break;
@@ -400,17 +399,17 @@ bool BytecodeParser::ParseInstruction(const uchar *&Buf, const uchar *EndBuf,
   }
   case Instruction::Store: {
     vector<Value*> Idx;
-    if (!isa<PointerType>(Raw.Ty)) return failure(true);
+    if (!isa<PointerType>(Raw.Ty)) return true;
     const CompositeType *TopTy = dyn_cast<CompositeType>(Raw.Ty);
 
     switch (Raw.NumOperands) {
     case 0: 
-    case 1: cerr << "Invalid store encountered!\n"; return failure(true);
+    case 1: cerr << "Invalid store encountered!\n"; return true;
     case 2: break;
     case 3:
-      if (!TopTy) return failure(true);
+      if (!TopTy) return true;
       Idx.push_back(V = getValue(TopTy->getIndexType(), Raw.Arg3));
-      if (!V) return failure(true);
+      if (!V) return true;
       break;
     default:
       vector<unsigned> &args = *Raw.VarArgs;
@@ -418,20 +417,20 @@ bool BytecodeParser::ParseInstruction(const uchar *&Buf, const uchar *EndBuf,
       unsigned i, E;
       for (i = 0, E = args.size(); ElTy && i != E; ++i) {
 	Idx.push_back(V = getValue(ElTy->getIndexType(), args[i]));
-	if (!V) return failure(true);
+	if (!V) return true;
 
         const Type *ETy = MemAccessInst::getIndexedType(Raw.Ty, Idx, true);
         ElTy = dyn_cast_or_null<CompositeType>(ETy);
       }
       if (i != E)
-        return failure(true);  // didn't use up all of the indices!
+        return true;  // didn't use up all of the indices!
 
       delete Raw.VarArgs; 
       break;
     }
 
     const Type *ElType = StoreInst::getIndexedType(Raw.Ty, Idx);
-    if (ElType == 0) return failure(true);
+    if (ElType == 0) return true;
     Res = new StoreInst(getValue(ElType, Raw.Arg1), getValue(Raw.Ty, Raw.Arg2),
 			Idx);
     return false;
@@ -440,5 +439,5 @@ bool BytecodeParser::ParseInstruction(const uchar *&Buf, const uchar *EndBuf,
 
   cerr << "Unrecognized instruction! " << Raw.Opcode 
        << " ADDR = 0x" << (void*)Buf << "\n";
-  return failure(true);
+  return true;
 }
diff --git a/lib/Bytecode/Reader/Reader.cpp b/lib/Bytecode/Reader/Reader.cpp
index 8d4ad56406..0e40625976 100644
--- a/lib/Bytecode/Reader/Reader.cpp
+++ b/lib/Bytecode/Reader/Reader.cpp
@@ -5,7 +5,7 @@
 // Note that this library should be as fast as possible, reentrant, and 
 // threadsafe!!
 //
-// TODO: Make error message outputs be configurable depending on an option?
+// TODO: Return error messages to caller instead of printing them out directly.
 // TODO: Allow passing in an option to ignore the symbol table
 //
 //===----------------------------------------------------------------------===//
@@ -13,7 +13,6 @@
 #include "ReaderInternals.h"
 #include "llvm/Bytecode/Reader.h"
 #include "llvm/Bytecode/Format.h"
-#include "llvm/GlobalVariable.h"
 #include "llvm/Module.h"
 #include "llvm/Constants.h"
 #include "llvm/iPHINode.h"
@@ -24,7 +23,6 @@
 #include <fcntl.h>
 #include <unistd.h>
 #include <algorithm>
-#include <iostream>
 using std::cerr;
 using std::pair;
 using std::make_pair;
@@ -56,14 +54,14 @@ const Type *BytecodeParser::getType(unsigned ID) {
   //cerr << "Looking up Type ID: " << ID << "\n";
 
   const Value *D = getValue(Type::TypeTy, ID, false);
-  if (D == 0) return failure<const Type*>(0);
+  if (D == 0) return 0;
 
   return cast<Type>(D);
 }
 
 int BytecodeParser::insertValue(Value *Val, std::vector<ValueList> &ValueTab) {
   unsigned type;
-  if (getTypeSlot(Val->getType(), type)) return failure<int>(-1);
+  if (getTypeSlot(Val->getType(), type)) return -1;
   assert(type != Type::TypeTyID && "Types should never be insertValue'd!");
  
   if (ValueTab.size() <= type)
@@ -80,7 +78,7 @@ Value *BytecodeParser::getValue(const Type *Ty, unsigned oNum, bool Create) {
   unsigned Num = oNum;
   unsigned type;   // The type plane it lives in...
 
-  if (getTypeSlot(Ty, type)) return failure<Value*>(0); // TODO: true
+  if (getTypeSlot(Ty, type)) return 0;
 
   if (type == Type::TypeTyID) {  // The 'type' plane has implicit values
     assert(Create == false);
@@ -111,22 +109,26 @@ Value *BytecodeParser::getValue(const Type *Ty, unsigned oNum, bool Create) {
   if (Values.size() > type && Values[type].size() > Num)
     return Values[type][Num];
 
-  if (!Create) return failure<Value*>(0);  // Do not create a placeholder?
+  if (!Create) return 0;  // Do not create a placeholder?
 
   Value *d = 0;
   switch (Ty->getPrimitiveID()) {
-  case Type::LabelTyID: d = new    BBPHolder(Ty, oNum); break;
   case Type::FunctionTyID:
     cerr << "Creating method pholder! : " << type << ":" << oNum << " " 
 	 << Ty->getName() << "\n";
-    d = new MethPHolder(Ty, oNum);
-    if (insertValue(d, LateResolveModuleValues) ==-1) return failure<Value*>(0);
+    d = new FunctionPHolder(Ty, oNum);
+    if (insertValue(d, LateResolveModuleValues) == -1) return 0;
     return d;
-  default:                   d = new   DefPHolder(Ty, oNum); break;
+  case Type::LabelTyID:
+    d = new BBPHolder(Ty, oNum);
+    break;
+  default:
+    d = new ValPHolder(Ty, oNum);
+    break;
   }
 
   assert(d != 0 && "How did we not make something?");
-  if (insertValue(d, LateResolveValues) == -1) return failure<Value*>(0);
+  if (insertValue(d, LateResolveValues) == -1) return 0;
   return d;
 }
 
@@ -168,11 +170,11 @@ bool BytecodeParser::ParseBasicBlock(const uchar *&Buf, const uchar *EndBuf,
     Instruction *Inst;
     if (ParseInstruction(Buf, EndBuf, Inst)) {
       delete BB;
-      return failure(true);
+      return true;
     }
 
-    if (Inst == 0) { delete BB; return failure(true); }
-    if (insertValue(Inst, Values) == -1) { delete BB; return failure(true); }
+    if (Inst == 0) { delete BB; return true; }
+    if (insertValue(Inst, Values) == -1) { delete BB; return true; }
 
     BB->getInstList().push_back(Inst);
 
@@ -188,9 +190,9 @@ bool BytecodeParser::ParseSymbolTable(const uchar *&Buf, const uchar *EndBuf,
     // Symtab block header: [num entries][type id number]
     unsigned NumEntries, Typ;
     if (read_vbr(Buf, EndBuf, NumEntries) ||
-        read_vbr(Buf, EndBuf, Typ)) return failure(true);
+        read_vbr(Buf, EndBuf, Typ)) return true;
     const Type *Ty = getType(Typ);
-    if (Ty == 0) return failure(true);
+    if (Ty == 0) return true;
 
     BCR_TRACE(3, "Plane Type: '" << Ty << "' with " << NumEntries <<
 	      " entries\n");
@@ -198,15 +200,15 @@ bool BytecodeParser::ParseSymbolTable(const uchar *&Buf, const uchar *EndBuf,
     for (unsigned i = 0; i < NumEntries; ++i) {
       // Symtab entry: [def slot #][name]
       unsigned slot;
-      if (read_vbr(Buf, EndBuf, slot)) return failure(true);
+      if (read_vbr(Buf, EndBuf, slot)) return true;
       std::string Name;
       if (read(Buf, EndBuf, Name, false))  // Not aligned...
-	return failure(true);
+	return true;
 
       Value *D = getValue(Ty, slot, false); // Find mapping...
       if (D == 0) {
 	BCR_TRACE(3, "FAILED LOOKUP: Slot #" << slot << "\n");
-	return failure(true);
+	return true;
       }
       BCR_TRACE(4, "Map: '" << Name << "' to #" << slot << ":" << D;
 		if (!isa<Instruction>(D)) cerr << "\n");
@@ -215,141 +217,53 @@ bool BytecodeParser::ParseSymbolTable(const uchar *&Buf, const uchar *EndBuf,
     }
   }
 
-  if (Buf > EndBuf) return failure(true);
+  if (Buf > EndBuf) return true;
   return false;
 }
 
-Value*
-ConstantFwdRefs::find(const Type* Ty, unsigned Slot) {
-  GlobalRefsType::iterator I = GlobalRefs.find(make_pair(Ty, Slot));
-  if (I != GlobalRefs.end()) {
-    return I->second;
-  } else {
-    return failure<Value*>(0);
-  }
-}
+void BytecodeParser::ResolveReferencesToValue(Value *NewV, unsigned Slot) {
+  GlobalRefsType::iterator I = GlobalRefs.find(make_pair(NewV->getType(),Slot));
+  if (I == GlobalRefs.end()) return;   // Never forward referenced?
 
-void
-ConstantFwdRefs::insert(const Type* Ty, unsigned Slot, Value* V) {
-  // Keep track of the fact that we have a forward ref to recycle it
-  const pair<GlobalRefsType::iterator, bool>& result =
-    GlobalRefs.insert(make_pair(make_pair(Ty, Slot), V));
-  assert(result.second == true && "Entry already exists for this slot?");
-}
+  BCR_TRACE(3, "Mutating forward refs!\n");
+  Value *VPH = I->second;   // Get the placeholder...
 
-void
-ConstantFwdRefs::erase(const Type* Ty, unsigned Slo