[OpaquePtr] BitcodeReader: don't rely on Types derived from a Value to provide pointer structure

[t.p.northover]

Summary

There are a handful of places where the result type of an instruction (or ConstantExpr) can depend on one of the input operands during bitcode parsing. We are and have been removing this from bitcode generated in the future, but will still need to be able to read and upgrade old bitcode when we move to opaque pointers.

The high-level effect of this is that BitcodeReader will need to derive the needed type information more or less directly from the type records (which will still contain the full structure of pointer types for old bitcode) rather than from some particular Value's Type, which may be dramatically flattened.

The future I imagine has a BitcodeSerializationPointerType which is only ever created by BitcodeReader.cpp, and then the flattenPointerTypes would recursively convert any given type to use the PointerType on demand, and that's what would end up in Value instances. But there are other ways to do it that don't fundamentally change the structure of this patch as far as I can tell.

I tested the libLTO.dylib with these changes when self-hosting with a clang binary back to 3.6 (before the load/GEP changes) and it works fine. Before that Clang's front-end chokes on something and segfaults; probably something too new in a system header but I didn't investigate.

[dblaikie]

I don't fully have the direction you've in mind/that this is a part of in my head - but roughly enough to give a thumbs-up here :)

[dexonsmith]

In D62742#1540426, @dblaikie wrote:

I don't fully have the direction you've in mind/that this is a part of in my head - but roughly enough to give a thumbs-up here :)

FWIW, I took a look back when Tim posted this and it looked roughly good to me too.

[t.p.northover]

Thanks. Should be committed as r364550.

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

class BitcodeReader : public BitcodeReaderBase, public GVMaterializer {
   uint64_t LastFunctionBlockBit = 0;
   bool SeenValueSymbolTable = false;
   uint64_t VSTOffset = 0;
 
   std::vector<std::string> SectionTable;
   std::vector<std::string> GCTable;
 
   std::vector<Type*> TypeList;
+  DenseMap<Function *, FunctionType *> FunctionTypes;
   BitcodeReaderValueList ValueList;
   Optional<MetadataLoader> MDLoader;
   std::vector<Comdat *> ComdatList;
   SmallVector<Instruction *, 64> InstructionList;
 
   std::vector<std::pair<GlobalVariable *, unsigned>> GlobalInits;
   std::vector<std::pair<GlobalIndirectSymbol *, unsigned>> IndirectSymbolInits;
   std::vector<std::pair<Function *, unsigned>> FunctionPrefixes;
public:
 
   void setStripDebugInfo() override;
 
 private:
   std::vector<StructType *> IdentifiedStructTypes;
   StructType *createIdentifiedStructType(LLVMContext &Context, StringRef Name);
   StructType *createIdentifiedStructType(LLVMContext &Context);
 
-  Type *getTypeByID(unsigned ID);
+  /// Map all pointer types within \param Ty to the opaque pointer
+  /// type in the same address space if opaque pointers are being
+  /// used, otherwise nop. This converts a bitcode-reader internal
+  /// type into one suitable for use in a Value.
+  Type *flattenPointerTypes(Type *Ty) {
+    return Ty;
+  }
+
+  /// Given a fully structured pointer type (i.e. not opaque), return
+  /// the flattened form of its element, suitable for use in a Value.
+  Type *getPointerElementFlatType(Type *Ty) {
+    return flattenPointerTypes(cast<PointerType>(Ty)->getElementType());
+  }
+
+  /// Given a fully structured pointer type, get its element type in
+  /// both fully structured form, and flattened form suitable for use
+  /// in a Value.
+  std::pair<Type *, Type *> getPointerElementTypes(Type *FullTy) {
+    Type *ElTy = cast<PointerType>(FullTy)->getElementType();
+    return std::make_pair(ElTy, flattenPointerTypes(ElTy));
+  }
+
+  /// Return the flattened type (suitable for use in a Value)
+  /// specified by the given \param ID.
+  Type *getTypeByID(unsigned ID) {
+    return flattenPointerTypes(getFullyStructuredTypeByID(ID));
+  }
 
-  Value *getFnValueByID(unsigned ID, Type *Ty) {
+  /// Return the fully structured (bitcode-reader internal) type
+  /// corresponding to the given \param ID.
+  Type *getFullyStructuredTypeByID(unsigned ID);
+
+  Value *getFnValueByID(unsigned ID, Type *Ty, Type **FullTy = nullptr) {
     if (Ty && Ty->isMetadataTy())
       return MetadataAsValue::get(Ty->getContext(), getFnMetadataByID(ID));
-    return ValueList.getValueFwdRef(ID, Ty);
+    return ValueList.getValueFwdRef(ID, Ty, FullTy);
   }
 
   Metadata *getFnMetadataByID(unsigned ID) {
     return MDLoader->getMetadataFwdRefOrLoad(ID);
   }
 
   BasicBlock *getBasicBlock(unsigned ID) const {
     if (ID >= FunctionBBs.size()) return nullptr; // Invalid ID
     return FunctionBBs[ID];
   }
 
   AttributeList getAttributes(unsigned i) const {
     if (i-1 < MAttributes.size())
       return MAttributes[i-1];
     return AttributeList();
   }
 
   /// Read a value/type pair out of the specified record from slot 'Slot'.
   /// Increment Slot past the number of slots used in the record. Return true on
   /// failure.
   bool getValueTypePair(SmallVectorImpl<uint64_t> &Record, unsigned &Slot,
-                        unsigned InstNum, Value *&ResVal) {
+                        unsigned InstNum, Value *&ResVal,
+                        Type **FullTy = nullptr) {
     if (Slot == Record.size()) return true;
     unsigned ValNo = (unsigned)Record[Slot++];
     // Adjust the ValNo, if it was encoded relative to the InstNum.
     if (UseRelativeIDs)
       ValNo = InstNum - ValNo;
     if (ValNo < InstNum) {
       // If this is not a forward reference, just return the value we already
       // have.
-      ResVal = getFnValueByID(ValNo, nullptr);
+      ResVal = getFnValueByID(ValNo, nullptr, FullTy);
       return ResVal == nullptr;
     }
     if (Slot == Record.size())
       return true;
 
     unsigned TypeNo = (unsigned)Record[Slot++];
     ResVal = getFnValueByID(ValNo, getTypeByID(TypeNo));
+    if (FullTy)
+      *FullTy = getFullyStructuredTypeByID(TypeNo);
     return ResVal == nullptr;
   }
 
   /// Read a value out of the specified record from slot 'Slot'. Increment Slot
   /// past the number of slots used by the value in the record. Return true if
   /// there is an error.
   bool popValue(SmallVectorImpl<uint64_t> &Record, unsigned &Slot,
                 unsigned InstNum, Type *Ty, Value *&ResVal) {
Value *getValueSigned(SmallVectorImpl<uint64_t> &Record, unsigned Slot,
     // Adjust the ValNo, if it was encoded relative to the InstNum.
     if (UseRelativeIDs)
       ValNo = InstNum - ValNo;
     return getFnValueByID(ValNo, Ty);
   }
 
   /// Upgrades old-style typeless byval attributes by adding the corresponding
   /// argument's pointee type.
-  void propagateByValTypes(CallBase *CB);
+  void propagateByValTypes(CallBase *CB, ArrayRef<Type *> ArgsFullTys);
 
   /// Converts alignment exponent (i.e. power of two (or zero)) to the
   /// corresponding alignment to use. If alignment is too large, returns
   /// a corresponding error code.
   Error parseAlignmentValue(uint64_t Exponent, unsigned &Alignment);
   Error parseAttrKind(uint64_t Code, Attribute::AttrKind *Kind);
   Error parseModule(uint64_t ResumeBit, bool ShouldLazyLoadMetadata = false);
 

 
 static void upgradeDLLImportExportLinkage(GlobalValue *GV, unsigned Val) {
   switch (Val) {
   case 5: GV->setDLLStorageClass(GlobalValue::DLLImportStorageClass); break;
   case 6: GV->setDLLStorageClass(GlobalValue::DLLExportStorageClass); break;
   }
 }
 
-Type *BitcodeReader::getTypeByID(unsigned ID) {
+Type *BitcodeReader::getFullyStructuredTypeByID(unsigned ID) {
   // The type table size is always specified correctly.
   if (ID >= TypeList.size())
     return nullptr;
 
   if (Type *Ty = TypeList[ID])
     return Ty;
 
   // If we have a forward reference, the only possible case is when it is to a

 Error BitcodeReader::parseConstants() {
   if (Stream.EnterSubBlock(bitc::CONSTANTS_BLOCK_ID))
     return error("Invalid record");
 
   SmallVector<uint64_t, 64> Record;
 
   // Read all the records for this value table.
   Type *CurTy = Type::getInt32Ty(Context);
+  Type *CurFullTy = Type::getInt32Ty(Context);
   unsigned NextCstNo = ValueList.size();
 
   while (true) {
     BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
 
     switch (Entry.Kind) {
     case BitstreamEntry::SubBlock: // Handled for us already.
     case BitstreamEntry::Error:
case bitc::CST_CODE_UNDEF:     // UNDEF
       break;
     case bitc::CST_CODE_SETTYPE:   // SETTYPE: [typeid]
       if (Record.empty())
         return error("Invalid record");
       if (Record[0] >= TypeList.size() || !TypeList[Record[0]])
         return error("Invalid record");
       if (TypeList[Record[0]] == VoidType)
         return error("Invalid constant type");
-      CurTy = TypeList[Record[0]];
+      CurFullTy = TypeList[Record[0]];
+      CurTy = flattenPointerTypes(CurFullTy);
       continue;  // Skip the ValueList manipulation.
     case bitc::CST_CODE_NULL:      // NULL
       V = Constant::getNullValue(CurTy);
       break;
     case bitc::CST_CODE_INTEGER:   // INTEGER: [intval]
       if (!CurTy->isIntegerTy() || Record.empty())
         return error("Invalid record");
       V = ConstantInt::get(CurTy, decodeSignRotatedValue(Record[0]));
case bitc::CST_CODE_CE_GEP_WITH_INRANGE_INDEX: { // [ty, flags, n x
       if (BitCode == bitc::CST_CODE_CE_GEP_WITH_INRANGE_INDEX) {
         uint64_t Op = Record[OpNum++];
         InBounds = Op & 1;
         InRangeIndex = Op >> 1;
       } else if (BitCode == bitc::CST_CODE_CE_INBOUNDS_GEP)
         InBounds = true;
 
       SmallVector<Constant*, 16> Elts;
+      Type *Elt0FullTy = nullptr;
       while (OpNum != Record.size()) {
+        if (!Elt0FullTy)
+          Elt0FullTy = getFullyStructuredTypeByID(Record[OpNum]);
         Type *ElTy = getTypeByID(Record[OpNum++]);
         if (!ElTy)
           return error("Invalid record");
         Elts.push_back(ValueList.getConstantFwdRef(Record[OpNum++], ElTy));
       }
 
       if (Elts.size() < 1)
         return error("Invalid gep with no operands");
 
       Type *ImplicitPointeeType =
-          cast<PointerType>(Elts[0]->getType()->getScalarType())
-              ->getElementType();
+          getPointerElementFlatType(Elt0FullTy->getScalarType());
       if (!PointeeType)
         PointeeType = ImplicitPointeeType;
       else if (PointeeType != ImplicitPointeeType)
         return error("Explicit gep operator type does not match pointee type "
                      "of pointer operand");
 
       ArrayRef<Constant *> Indices(Elts.begin() + 1, Elts.end());
       V = ConstantExpr::getGetElementPtr(PointeeType, Elts[0], Indices,
case bitc::CST_CODE_INLINEASM_OLD: {
       unsigned ConstStrSize = Record[2+AsmStrSize];
       if (3+AsmStrSize+ConstStrSize > Record.size())
         return error("Invalid record");
 
       for (unsigned i = 0; i != AsmStrSize; ++i)
         AsmStr += (char)Record[2+i];
       for (unsigned i = 0; i != ConstStrSize; ++i)
         ConstrStr += (char)Record[3+AsmStrSize+i];
-      PointerType *PTy = cast<PointerType>(CurTy);
       UpgradeInlineAsmString(&AsmStr);
-      V = InlineAsm::get(cast<FunctionType>(PTy->getElementType()),
-                         AsmStr, ConstrStr, HasSideEffects, IsAlignStack);
+      V = InlineAsm::get(
+          cast<FunctionType>(getPointerElementFlatType(CurFullTy)), AsmStr,
+          ConstrStr, HasSideEffects, IsAlignStack);
       break;
     }
     // This version adds support for the asm dialect keywords (e.g.,
     // inteldialect).
     case bitc::CST_CODE_INLINEASM: {
       if (Record.size() < 2)
         return error("Invalid record");
       std::string AsmStr, ConstrStr;
       bool HasSideEffects = Record[0] & 1;
       bool IsAlignStack = (Record[0] >> 1) & 1;
       unsigned AsmDialect = Record[0] >> 2;
       unsigned AsmStrSize = Record[1];
       if (2+AsmStrSize >= Record.size())
         return error("Invalid record");
       unsigned ConstStrSize = Record[2+AsmStrSize];
       if (3+AsmStrSize+ConstStrSize > Record.size())
         return error("Invalid record");
 
       for (unsigned i = 0; i != AsmStrSize; ++i)
         AsmStr += (char)Record[2+i];
       for (unsigned i = 0; i != ConstStrSize; ++i)
         ConstrStr += (char)Record[3+AsmStrSize+i];
-      PointerType *PTy = cast<PointerType>(CurTy);
       UpgradeInlineAsmString(&AsmStr);
-      V = InlineAsm::get(cast<FunctionType>(PTy->getElementType()),
-                         AsmStr, ConstrStr, HasSideEffects, IsAlignStack,
-                         InlineAsm::AsmDialect(AsmDialect));
+      V = InlineAsm::get(
+          cast<FunctionType>(getPointerElementFlatType(CurFullTy)), AsmStr,
+          ConstrStr, HasSideEffects, IsAlignStack,
       break;
     }
     case bitc::CST_CODE_BLOCKADDRESS:{
       if (Record.size() < 3)
         return error("Invalid record");
       Type *FnTy = getTypeByID(Record[0]);
       if (!FnTy)
         return error("Invalid record");
case bitc::CST_CODE_BLOCKADDRESS:{
           FwdBBs[BBID] = BasicBlock::Create(Context);
         BB = FwdBBs[BBID];
       }
       V = BlockAddress::get(Fn, BB);
       break;
     }
     }
 
-    ValueList.assignValue(V, NextCstNo);
+    assert(V->getType() == flattenPointerTypes(CurFullTy) &&
+           "Incorrect fully structured type provided for Constant");
+    ValueList.assignValue(V, NextCstNo, CurFullTy);
     ++NextCstNo;
   }
 }
 
 Error BitcodeReader::parseUseLists() {
   if (Stream.EnterSubBlock(bitc::USELIST_BLOCK_ID))
     return error("Invalid record");
 
Error BitcodeReader::parseGlobalVarRecord(ArrayRef<uint64_t> Record) {
   // dllstorageclass, comdat, attributes, preemption specifier,
   // partition strtab offset, partition strtab size] (name in VST)
   // v2: [strtab_offset, strtab_size, v1]
   StringRef Name;
   std::tie(Name, Record) = readNameFromStrtab(Record);
 
   if (Record.size() < 6)
     return error("Invalid record");
-  Type *Ty = getTypeByID(Record[0]);
+  Type *FullTy = getFullyStructuredTypeByID(Record[0]);
+  Type *Ty = flattenPointerTypes(FullTy);
   if (!Ty)
     return error("Invalid record");
   bool isConstant = Record[1] & 1;
   bool explicitType = Record[1] & 2;
   unsigned AddressSpace;
   if (explicitType) {
     AddressSpace = Record[1] >> 2;
   } else {
     if (!Ty->isPointerTy())
       return error("Invalid type for value");
     AddressSpace = cast<PointerType>(Ty)->getAddressSpace();
-    Ty = cast<PointerType>(Ty)->getElementType();
+    std::tie(FullTy, Ty) = getPointerElementTypes(FullTy);
   }
 
   uint64_t RawLinkage = Record[3];
   GlobalValue::LinkageTypes Linkage = getDecodedLinkage(RawLinkage);
   unsigned Alignment;
   if (Error Err = parseAlignmentValue(Record[4], Alignment))
     return Err;
   std::string Section;
Error BitcodeReader::parseGlobalVarRecord(ArrayRef<uint64_t> Record) {
   NewGV->setVisibility(Visibility);
   NewGV->setUnnamedAddr(UnnamedAddr);
 
   if (Record.size() > 10)
     NewGV->setDLLStorageClass(getDecodedDLLStorageClass(Record[10]));
   else
     upgradeDLLImportExportLinkage(NewGV, RawLinkage);
 
-  ValueList.push_back(NewGV);
+  FullTy = PointerType::get(FullTy, AddressSpace);
+  assert(NewGV->getType() == flattenPointerTypes(FullTy) &&
+         "Incorrect fully specified type for GlobalVariable");
+  ValueList.push_back(NewGV, FullTy);
 
   // Remember which value to use for the global initializer.
   if (unsigned InitID = Record[2])
     GlobalInits.push_back(std::make_pair(NewGV, InitID - 1));
 
   if (Record.size() > 11) {
     if (unsigned ComdatID = Record[11]) {
       if (ComdatID > ComdatList.size())
Error BitcodeReader::parseFunctionRecord(ArrayRef<uint64_t> Record) {
   // visibility, gc, unnamed_addr, prologuedata, dllstorageclass, comdat,
   // prefixdata,  personalityfn, preemption specifier, addrspace] (name in VST)
   // v2: [strtab_offset, strtab_size, v1]
   StringRef Name;
   std::tie(Name, Record) = readNameFromStrtab(Record);
 
   if (Record.size() < 8)
     return error("Invalid record");
-  Type *Ty = getTypeByID(Record[0]);
-  if (!Ty)
-    return error("Invalid record");
-  if (auto *PTy = dyn_cast<PointerType>(Ty))
-    Ty = PTy->getElementType();
-  auto *FTy = dyn_cast<FunctionType>(Ty);
+  Type *FullFTy = getFullyStructuredTypeByID(Record[0]);
+  Type *FTy = flattenPointerTypes(FullFTy);
   if (!FTy)
+    return error("Invalid record");
+  if (isa<PointerType>(FTy))
+    std::tie(FullFTy, FTy) = getPointerElementTypes(FullFTy);
+
+  if (!isa<FunctionType>(FTy))
     return error("Invalid type for value");
   auto CC = static_cast<CallingConv::ID>(Record[1]);
   if (CC & ~CallingConv::MaxID)
     return error("Invalid calling convention ID");
 
   unsigned AddrSpace = TheModule->getDataLayout().getProgramAddressSpace();
   if (Record.size() > 16)
     AddrSpace = Record[16];
 
-  Function *Func = Function::Create(FTy, GlobalValue::ExternalLinkage,
-                                    AddrSpace, Name, TheModule);
+  Function *Func =
+      Function::Create(cast<FunctionType>(FTy), GlobalValue::ExternalLinkage,
 
+  assert(Func->getFunctionType() == flattenPointerTypes(FullFTy) &&
+         "Incorrect fully specified type provided for function");
+  FunctionTypes[Func] = cast<FunctionType>(FullFTy);
+
   Func->setCallingConv(CC);
   bool isProto = Record[2];
   uint64_t RawLinkage = Record[3];
   Func->setLinkage(getDecodedLinkage(RawLinkage));
   Func->setAttributes(getAttributes(Record[4]));
 
   // Upgrade any old-style byval without a type by propagating the argument's
   // pointee type. There should be no opaque pointers where the byval type is
   // implicit.
-  for (auto &Arg : Func->args()) {
-    if (Arg.hasByValAttr() && !Arg.getParamByValType()) {
-      Arg.removeAttr(Attribute::ByVal);
-      Arg.addAttr(Attribute::getWithByValType(
-          Context, Arg.getType()->getPointerElementType()));
-    }
+  for (unsigned i = 0; i != Func->arg_size(); ++i) {
+    if (!Func->hasParamAttribute(i, Attribute::ByVal))
+      continue;
+
+    Type *PTy = cast<FunctionType>(FullFTy)->getParamType(i);
+    Func->removeParamAttr(i, Attribute::ByVal);
+    Func->addParamAttr(i, Attribute::getWithByValType(
+                              Context, getPointerElementFlatType(PTy)));
   }
 
   unsigned Alignment;
   if (Error Err = parseAlignmentValue(Record[5], Alignment))
     return Err;
   Func->setAlignment(Alignment);
   if (Record[6]) {
     if (Record[6] - 1 >= SectionTable.size())
Error BitcodeReader::parseFunctionRecord(ArrayRef<uint64_t> Record) {
   inferDSOLocal(Func);
 
   // Record[16] is the address space number.
 
   // Check whether we have enough values to read a partition name.
   if (Record.size() > 18)
     Func->setPartition(StringRef(Strtab.data() + Record[17], Record[18]));
 
-  ValueList.push_back(Func);
+  Type *FullTy = PointerType::get(FullFTy, AddrSpace);
+  assert(Func->getType() == flattenPointerTypes(FullTy) &&
+         "Incorrect fully specified type provided for Function");
+  ValueList.push_back(Func, FullTy);
 
   // If this is a function with a body, remember the prototype we are
   // creating now, so that we can match up the body with them later.
   if (!isProto) {
     Func->setIsMaterializable(true);
     FunctionsWithBodies.push_back(Func);
     DeferredFunctionInfo[Func] = 0;
   }
Error BitcodeReader::parseGlobalIndirectSymbolRecord(
   // v2: [strtab_offset, strtab_size, v1]
   StringRef Name;
   std::tie(Name, Record) = readNameFromStrtab(Record);
 
   bool NewRecord = BitCode != bitc::MODULE_CODE_ALIAS_OLD;
   if (Record.size() < (3 + (unsigned)NewRecord))
     return error("Invalid record");
   unsigned OpNum = 0;
-  Type *Ty = getTypeByID(Record[OpNum++]);
+  Type *FullTy = getFullyStructuredTypeByID(Record[OpNum++]);
+  Type *Ty = flattenPointerTypes(FullTy);
   if (!Ty)
     return error("Invalid record");
 
   unsigned AddrSpace;
   if (!NewRecord) {
     auto *PTy = dyn_cast<PointerType>(Ty);
     if (!PTy)
       return error("Invalid type for value");
-    Ty = PTy->getElementType();
+    std::tie(FullTy, Ty) = getPointerElementTypes(FullTy);
     AddrSpace = PTy->getAddressSpace();
   } else {
     AddrSpace = Record[OpNum++];
   }
 
   auto Val = Record[OpNum++];
   auto Linkage = Record[OpNum++];
   GlobalIndirectSymbol *NewGA;
   if (BitCode == bitc::MODULE_CODE_ALIAS ||
       BitCode == bitc::MODULE_CODE_ALIAS_OLD)
     NewGA = GlobalAlias::create(Ty, AddrSpace, getDecodedLinkage(Linkage), Name,
                                 TheModule);
   else
     NewGA = GlobalIFunc::create(Ty, AddrSpace, getDecodedLinkage(Linkage), Name,
                                 nullptr, TheModule);
+
+  assert(NewGA->getValueType() == flattenPointerTypes(FullTy) &&
+         "Incorrect fully structured type provided for GlobalIndirectSymbol");
   // Old bitcode files didn't have visibility field.
   // Local linkage must have default visibility.
   if (OpNum != Record.size()) {
     auto VisInd = OpNum++;
     if (!NewGA->hasLocalLinkage())
       // FIXME: Change to an error if non-default in 4.0.
       NewGA->setVisibility(getDecodedVisibility(Record[VisInd]));
   }
Error BitcodeReader::parseGlobalIndirectSymbolRecord(
 
   // Check whether we have enough values to read a partition name.
   if (OpNum + 1 < Record.size()) {
     NewGA->setPartition(
         StringRef(Strtab.data() + Record[OpNum], Record[OpNum + 1]));
     OpNum += 2;
   }
 
-  ValueList.push_back(NewGA);
+  FullTy = PointerType::get(FullTy, AddrSpace);
+  assert(NewGA->getType() == flattenPointerTypes(FullTy) &&
+         "Incorrect fully structured type provided for GlobalIndirectSymbol");
+  ValueList.push_back(NewGA, FullTy);
   IndirectSymbolInits.push_back(std::make_pair(NewGA, Val));
   return Error::success();
 }
 
 Error BitcodeReader::parseModule(uint64_t ResumeBit,
                                  bool ShouldLazyLoadMetadata) {
   if (ResumeBit)
     Stream.JumpToBit(ResumeBit);
Error BitcodeReader::typeCheckLoadStoreInst(Type *ValType, Type *PtrType) {
   if (ValType && ValType != ElemType)
     return error("Explicit load/store type does not match pointee "
                  "type of pointer operand");
   if (!PointerType::isLoadableOrStorableType(ElemType))
     return error("Cannot load/store from pointer");
   return Error::success();
 }
 
-void BitcodeReader::propagateByValTypes(CallBase *CB) {
-  for (unsigned i = 0; i < CB->getNumArgOperands(); ++i) {
-    if (CB->paramHasAttr(i, Attribute::ByVal) &&
-        !CB->getAttribute(i, Attribute::ByVal).getValueAsType()) {
-      CB->removeParamAttr(i, Attribute::ByVal);
-      CB->addParamAttr(
-          i, Attribute::getWithByValType(
-                 Context,
-                 CB->getArgOperand(i)->getType()->getPointerElementType()));
-    }
+void BitcodeReader::propagateByValTypes(CallBase *CB,
+                                        ArrayRef<Type *> ArgsFullTys) {
+  for (unsigned i = 0; i != CB->arg_size(); ++i) {
+    if (!CB->paramHasAttr(i, Attribute::ByVal))
+      continue;
+
+               Context, getPointerElementFlatType(ArgsFullTys[i])));
   }
 }
 
 /// Lazily parse the specified function body block.
 Error BitcodeReader::parseFunctionBody(Function *F) {
   if (Stream.EnterSubBlock(bitc::FUNCTION_BLOCK_ID))
     return error("Invalid record");
 
   // Unexpected unresolved metadata when parsing function.
   if (MDLoader->hasFwdRefs())
     return error("Invalid function metadata: incoming forward references");
 
   InstructionList.clear();
   unsigned ModuleValueListSize = ValueList.size();
   unsigned ModuleMDLoaderSize = MDLoader->size();
 
   // Add all the function arguments to the value table.
-  for (Argument &I : F->args())
-    ValueList.push_back(&I);
-
+  unsigned ArgNo = 0;
+  FunctionType *FullFTy = FunctionTypes[F];
+  for (Argument &I : F->args()) {
+    assert(I.getType() == flattenPointerTypes(FullFTy->getParamType(ArgNo)) &&
+           "Incorrect fully specified type for Function Argument");
+    ValueList.push_back(&I, FullFTy->getParamType(ArgNo++));
+  }
   unsigned NextValueNo = ValueList.size();
   BasicBlock *CurBB = nullptr;
   unsigned CurBBNo = 0;
 
   DebugLoc LastLoc;
   auto getLastInstruction = [&]() -> Instruction * {
     if (CurBB && !CurBB->empty())
       return &CurBB->back();
while (true) {
     case BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // Read a record.
     Record.clear();
     Instruction *I = nullptr;
+    Type *FullTy = nullptr;
     unsigned BitCode = Stream.readRecord(Entry.ID, Record);
     switch (BitCode) {
     default: // Default behavior: reject
       return error("Invalid value");
     case bitc::FUNC_CODE_DECLAREBLOCKS: {   // DECLAREBLOCKS: [nblocks]
       if (Record.size() < 1 || Record[0] == 0)
         return error("Invalid record");
       // Create all the basic blocks for the function.
while (true) {
     }
     case bitc::FUNC_CODE_INST_CAST: {    // CAST: [opval, opty, destty, castopc]
       unsigned OpNum = 0;
       Value *Op;
       if (getValueTypePair(Record, OpNum, NextValueNo, Op) ||
           OpNum+2 != Record.size())
         return error("Invalid record");
 
-      Type *ResTy = getTypeByID(Record[OpNum]);
+      FullTy = getFullyStructuredTypeByID(Record[OpNum]);
+      Type *ResTy = flattenPointerTypes(FullTy);
       int Opc = getDecodedCastOpcode(Record[OpNum + 1]);
       if (Opc == -1 || !ResTy)
         return error("Invalid record");
       Instruction *Temp = nullptr;
       if ((I = UpgradeBitCastInst(Opc, Op, ResTy, Temp))) {
         if (Temp) {
           InstructionList.push_back(Temp);
           CurBB->getInstList().push_back(Temp);
while (true) {
     case bitc::FUNC_CODE_INST_GEP: { // GEP: type, [n x operands]
       unsigned OpNum = 0;
 
       Type *Ty;
       bool InBounds;
 
       if (BitCode == bitc::FUNC_CODE_INST_GEP) {
         InBounds = Record[OpNum++];
-        Ty = getTypeByID(Record[OpNum++]);
+        FullTy = getFullyStructuredTypeByID(Record[OpNum++]);
+        Ty = flattenPointerTypes(FullTy);
       } else {
         InBounds = BitCode == bitc::FUNC_CODE_INST_INBOUNDS_GEP_OLD;
         Ty = nullptr;
       }
 
       Value *BasePtr;
-      if (getValueTypePair(Record, OpNum, NextValueNo, BasePtr))
+      Type *FullBaseTy = nullptr;
+      if (getValueTypePair(Record, OpNum, NextValueNo, BasePtr, &FullBaseTy))
         return error("Invalid record");
 
-      if (!Ty)
-        Ty = cast<PointerType>(BasePtr->getType()->getScalarType())
-                 ->getElementType();
-      else if (Ty !=
-               cast<PointerType>(BasePtr->getType()->getScalarType())
-                   ->getElementType())
+      if (!Ty) {
+        std::tie(FullTy, Ty) =
+            getPointerElementTypes(FullBaseTy->getScalarType());
+      } else if (Ty != getPointerElementFlatType(FullBaseTy->getScalarType()))
         return error(
             "Explicit gep type does not match pointee type of pointer operand");
 
       SmallVector<Value*, 16> GEPIdx;
       while (OpNum != Record.size()) {
         Value *Op;
         if (getValueTypePair(Record, OpNum, NextValueNo, Op))
           return error("Invalid record");
         GEPIdx.push_back(Op);
       }
 
       I = GetElementPtrInst::Create(Ty, BasePtr, GEPIdx);
+      FullTy = GetElementPtrInst::getGEPReturnType(FullTy, I, GEPIdx);
 
       InstructionList.push_back(I);
       if (InBounds)
         cast<GetElementPtrInst>(I)->setIsInBounds(true);
       break;
     }
 
     case bitc::FUNC_CODE_INST_EXTRACTVAL: {
                                        // EXTRACTVAL: [opty, opval, n x indices]
       unsigned OpNum = 0;
       Value *Agg;
-      if (getValueTypePair(Record, OpNum, NextValueNo, Agg))
+      if (getValueTypePair(Record, OpNum, NextValueNo, Agg, &FullTy))
         return error("Invalid record");
 
       unsigned RecSize = Record.size();
       if (OpNum == RecSize)
         return error("EXTRACTVAL: Invalid instruction with 0 indices");
 
       SmallVector<unsigned, 4> EXTRACTVALIdx;
-      Type *CurTy = Agg->getType();
       for (; OpNum != RecSize; ++OpNum) {
-        bool IsArray = CurTy->isArrayTy();
-        bool IsStruct = CurTy->isStructTy();
+        bool IsArray = FullTy->isArrayTy();
+        bool IsStruct = FullTy->isStructTy();
         uint64_t Index = Record[OpNum];
 
         if (!IsStruct && !IsArray)
           return error("EXTRACTVAL: Invalid type");
         if ((unsigned)Index != Index)
           return error("Invalid value");
-        if (IsStruct && Index >= CurTy->getStructNumElements())
+        if (IsStruct && Index >= FullTy->getStructNumElements())
           return error("EXTRACTVAL: Invalid struct index");
-        if (IsArray && Index >= CurTy->getArrayNumElements())
+        if (IsArray && Index >= FullTy->getArrayNumElements())
           return error("EXTRACTVAL: Invalid array index");
         EXTRACTVALIdx.push_back((unsigned)Index);
 
         if (IsStruct)
-          CurTy = CurTy->getStructElementType(Index);
+          FullTy = FullTy->getStructElementType(Index);
         else
-          CurTy = CurTy->getArrayElementType();
+          FullTy = FullTy->getArrayElementType();
       }
 
       I = ExtractValueInst::Create(Agg, EXTRACTVALIdx);
       InstructionList.push_back(I);
       break;
     }
 
     case bitc::FUNC_CODE_INST_INSERTVAL: {
                            // INSERTVAL: [opty, opval, opty, opval, n x indices]
       unsigned OpNum = 0;
       Value *Agg;
-      if (getValueTypePair(Record, OpNum, NextValueNo, Agg))
+      if (getValueTypePair(Record, OpNum, NextValueNo, Agg, &FullTy))
         return error("Invalid record");
       Value *Val;
       if (getValueTypePair(Record, OpNum, NextValueNo, Val))
         return error("Invalid record");
 
       unsigned RecSize = Record.size();
       if (OpNum == RecSize)
         return error("INSERTVAL: Invalid instruction with 0 indices");
case bitc::FUNC_CODE_INST_INSERTVAL: {
       break;
     }
 
     case bitc::FUNC_CODE_INST_SELECT: { // SELECT: [opval, ty, opval, opval]
       // obsolete form of select
       // handles select i1 ... in old bitcode
       unsigned OpNum = 0;
       Value *TrueVal, *FalseVal, *Cond;
-      if (getValueTypePair(Record, OpNum, NextValueNo, TrueVal) ||
+      if (getValueTypePair(Record, OpNum, NextValueNo, TrueVal, &FullTy) ||
           popValue(Record, OpNum, NextValueNo, TrueVal->getType(), FalseVal) ||
           popValue(Record, OpNum, NextValueNo, Type::getInt1Ty(Context), Cond))
         return error("Invalid record");
 
       I = SelectInst::Create(Cond, TrueVal, FalseVal);
       InstructionList.push_back(I);
       break;
     }
 
     case bitc::FUNC_CODE_INST_VSELECT: {// VSELECT: [ty,opval,opval,predty,pred]
       // new form of select
       // handles select i1 or select [N x i1]
       unsigned OpNum = 0;
       Value *TrueVal, *FalseVal, *Cond;
-      if (getValueTypePair(Record, OpNum, NextValueNo, TrueVal) ||
+      if (getValueTypePair(Record, OpNum, NextValueNo, TrueVal, &FullTy) ||
           popValue(Record, OpNum, NextValueNo, TrueVal->getType(), FalseVal) ||
           getValueTypePair(Record, OpNum, NextValueNo, Cond))
         return error("Invalid record");
 
       // select condition can be either i1 or [N x i1]
       if (VectorType* vector_type =
           dyn_cast<VectorType>(Cond->getType())) {
         // expect <n x i1>
case bitc::FUNC_CODE_INST_VSELECT: {// VSELECT: [ty,opval,opval,predty,pred]
           I->setFastMathFlags(FMF);
       }
       break;
     }
 
     case bitc::FUNC_CODE_INST_EXTRACTELT: { // EXTRACTELT: [opty, opval, opval]
       unsigned OpNum = 0;
       Value *Vec, *Idx;
-      if (getValueTypePair(Record, OpNum, NextValueNo, Vec) ||
+      if (getValueTypePair(Record, OpNum, NextValueNo, Vec, &FullTy) ||
           getValueTypePair(Record, OpNum, NextValueNo, Idx))
         return error("Invalid record");
       if (!Vec->getType()->isVectorTy())
         return error("Invalid type for value");
       I = ExtractElementInst::Create(Vec, Idx);
+      FullTy = FullTy->getVectorElementType();
       InstructionList.push_back(I);
       break;
     }
 
     case bitc::FUNC_CODE_INST_INSERTELT: { // INSERTELT: [ty, opval,opval,opval]
       unsigned OpNum = 0;
       Value *Vec, *Elt, *Idx;
-      if (getValueTypePair(Record, OpNum, NextValueNo, Vec))
+      if (getValueTypePair(Record, OpNum, NextValueNo, Vec, &FullTy))
         return error("Invalid record");
       if (!Vec->getType()->isVectorTy())
         return error("Invalid type for value");
       if (popValue(Record, OpNum, NextValueNo,
                    cast<VectorType>(Vec->getType())->getElementType(), Elt) ||
           getValueTypePair(Record, OpNum, NextValueNo, Idx))
         return error("Invalid record");
       I = InsertElementInst::Create(Vec, Elt, Idx);
       InstructionList.push_back(I);
       break;
     }
 
     case bitc::FUNC_CODE_INST_SHUFFLEVEC: {// SHUFFLEVEC: [opval,ty,opval,opval]
       unsigned OpNum = 0;
       Value *Vec1, *Vec2, *Mask;
-      if (getValueTypePair(Record, OpNum, NextValueNo, Vec1) ||
+      if (getValueTypePair(Record, OpNum, NextValueNo, Vec1, &FullTy) ||
           popValue(Record, OpNum, NextValueNo, Vec1->getType(), Vec2))
         return error("Invalid record");
 
       if (getValueTypePair(Record, OpNum, NextValueNo, Mask))
         return error("Invalid record");
       if (!Vec1->getType()->isVectorTy() || !Vec2->getType()->isVectorTy())
         return error("Invalid type for value");
       I = new ShuffleVectorInst(Vec1, Vec2, Mask);
+      FullTy = VectorType::get(FullTy->getVectorElementType(),
+                               Mask->getType()->getVectorNumElements());
       InstructionList.push_back(I);
       break;
     }
 
     case bitc::FUNC_CODE_INST_CMP:   // CMP: [opty, opval, opval, pred]
       // Old form of ICmp/FCmp returning bool
       // Existed to differentiate between icmp/fcmp and vicmp/vfcmp which were
       // both legal on vectors but had different behaviour.
case bitc::FUNC_CODE_INST_INVOKE: {
         return error("Invalid record");
       unsigned OpNum = 0;
       AttributeList PAL = getAttributes(Record[OpNum++]);
       unsigned CCInfo = Record[OpNum++];
       BasicBlock *NormalBB = getBasicBlock(Record[OpNum++]);
       BasicBlock *UnwindBB = getBasicBlock(Record[OpNum++]);
 
       FunctionType *FTy = nullptr;
-      if (CCInfo >> 13 & 1 &&
-          !(FTy = dyn_cast<FunctionType>(getTypeByID(Record[OpNum++]))))
+      FunctionType *FullFTy = nullptr;
+      if ((CCInfo >> 13) & 1) {
+        FullFTy =
+            dyn_cast<FunctionType>(getFullyStructuredTypeByID(Record[OpNum++]));
+        if (!FullFTy)
           return error("Explicit invoke type is not a function type");
+        FTy = cast<FunctionType>(flattenPointerTypes(FullFTy));
+      }
 
       Value *Callee;
-      if (getValueTypePair(Record, OpNum, NextValueNo, Callee))
+      if (getValueTypePair(Record, OpNum, NextValueNo, Callee, &FullTy))
         return error("Invalid record");
 
       PointerType *CalleeTy = dyn_cast<PointerType>(Callee->getType());
       if (!CalleeTy)
         return error("Callee is not a pointer");
       if (!FTy) {
-        FTy = dyn_cast<FunctionType>(CalleeTy->getElementType());
-        if (!FTy)
+        FullFTy =
+            dyn_cast<FunctionType>(cast<PointerType>(FullTy)->getElementType());
+        if (!FullFTy)
           return error("Callee is not of pointer to function type");
-      } else if (CalleeTy->getElementType() != FTy)
+        FTy = cast<FunctionType>(flattenPointerTypes(FullFTy));
+      } else if (getPointerElementFlatType(FullTy) != FTy)
         return error("Explicit invoke type does not match pointee type of "
                      "callee operand");
       if (Record.size() < FTy->getNumParams() + OpNum)
         return error("Insufficient operands to call");
 
       SmallVector<Value*, 16> Ops;
+      SmallVector<Type *, 16> ArgsFullTys;
       for (unsigned i = 0, e = FTy->getNumParams(); i != e; ++i, ++OpNum) {
         Ops.push_back(getValue(Record, OpNum, NextValueNo,
                                FTy->getParamType(i)));
+        ArgsFullTys.push_back(FullFTy->getParamType(i));
         if (!Ops.back())
           return error("Invalid record");
       }
 
       if (!FTy->isVarArg()) {
         if (Record.size() != OpNum)
           return error("Invalid record");
       } else {
         // Read type/value pairs for varargs params.
         while (OpNum != Record.size()) {
           Value *Op;
-          if (getValueTypePair(Record, OpNum, NextValueNo, Op))
+          Type *FullTy;
+          if (getValueTypePair(Record, OpNum, NextValueNo, Op, &FullTy))
             return error("Invalid record");
           Ops.push_back(Op);
+          ArgsFullTys.push_back(FullTy);
         }
       }
 
       I = InvokeInst::Create(FTy, Callee, NormalBB, UnwindBB, Ops,
                              OperandBundles);
+      FullTy = FullFTy->getReturnType();
       OperandBundles.clear();
       InstructionList.push_back(I);
       cast<InvokeInst>(I)->setCallingConv(
           static_cast<CallingConv::ID>(CallingConv::MaxID & CCInfo));
       cast<InvokeInst>(I)->setAttributes(PAL);
-      propagateByValTypes(cast<CallBase>(I));
+      propagateByValTypes(cast<CallBase>(I), ArgsFullTys);
 
       break;
     }
     case bitc::FUNC_CODE_INST_RESUME: { // RESUME: [opval]
       unsigned Idx = 0;
       Value *Val = nullptr;
       if (getValueTypePair(Record, Idx, NextValueNo, Val))
         return error("Invalid record");
case bitc::FUNC_CODE_INST_CALLBR: {
 
       BasicBlock *DefaultDest = getBasicBlock(Record[OpNum++]);
       unsigned NumIndirectDests = Record[OpNum++];
       SmallVector<BasicBlock *, 16> IndirectDests;
       for (unsigned i = 0, e = NumIndirectDests; i != e; ++i)
         IndirectDests.push_back(getBasicBlock(Record[OpNum++]));
 
       FunctionType *FTy = nullptr;
-      if (CCInfo >> bitc::CALL_EXPLICIT_TYPE & 1 &&
-          !(FTy = dyn_cast<FunctionType>(getTypeByID(Record[OpNum++]))))
+      FunctionType *FullFTy = nullptr;
+      if ((CCInfo >> bitc::CALL_EXPLICIT_TYPE) & 1) {
+        FullFTy =
+            dyn_cast<FunctionType>(getFullyStructuredTypeByID(Record[OpNum++]));
+        if (!FullFTy)
           return error("Explicit call type is not a function type");
+        FTy = cast<FunctionType>(flattenPointerTypes(FullFTy));
+      }
 
       Value *Callee;
-      if (getValueTypePair(Record, OpNum, NextValueNo, Callee))
+      if (getValueTypePair(Record, OpNum, NextValueNo, Callee, &FullTy))
         return error("Invalid record");
 
       PointerType *OpTy = dyn_cast<PointerType>(Callee->getType());
       if (!OpTy)
         return error("Callee is not a pointer type");
       if (!FTy) {
-        FTy = dyn_cast<FunctionType>(OpTy->getElementType());
-        if (!FTy)
+        FullFTy =
+            dyn_cast<FunctionType>(cast<PointerType>(FullTy)->getElementType());
+        if (!FullFTy)
           return error("Callee is not of pointer to function type");
-      } else if (OpTy->getElementType() != FTy)
+        FTy = cast<FunctionType>(flattenPointerTypes(FullFTy));
+      } else if (getPointerElementFlatType(FullTy) != FTy)
         return error("Explicit call type does not match pointee type of "
                      "callee operand");
       if (Record.size() < FTy->getNumParams() + OpNum)
         return error("Insufficient operands to call");
 
       SmallVector<Value*, 16> Args;
       // Read the fixed params.
       for (unsigned i = 0, e = FTy->getNumParams(); i != e; ++i, ++OpNum) {
case bitc::FUNC_CODE_INST_CALLBR: {
           if (getValueTypePair(Record, OpNum, NextValueNo, Op))
             return error("Invalid record");
           Args.push_back(Op);
         }
       }
 
       I = CallBrInst::Create(FTy, Callee, DefaultDest, IndirectDests, Args,
                              OperandBundles);
+      FullTy = FullFTy->getReturnType();
       OperandBundles.clear();
       InstructionList.push_back(I);
       cast<CallBrInst>(I)->setCallingConv(
           static_cast<CallingConv::ID>((0x7ff & CCInfo) >> bitc::CALL_CCONV));
       cast<CallBrInst>(I)->setAttributes(PAL);
       break;
     }
     case bitc::FUNC_CODE_INST_UNREACHABLE: // UNREACHABLE
       I = new UnreachableInst(Context);
       InstructionList.push_back(I);
       break;
     case bitc::FUNC_CODE_INST_PHI: { // PHI: [ty, val0,bb0, ...]
       if (Record.size() < 1 || ((Record.size()-1)&1))
         return error("Invalid record");
-      Type *Ty = getTypeByID(Record[0]);
+      FullTy = getFullyStructuredTypeByID(Record[0]);
+      Type *Ty = flattenPointerTypes(FullTy);
       if (!Ty)
         return error("Invalid record");
 
       PHINode *PN = PHINode::Create(Ty, (Record.size()-1)/2);
       InstructionList.push_back(PN);
 
       for (unsigned i = 0, e = Record.size()-1; i != e; i += 2) {
         Value *V;
case bitc::FUNC_CODE_INST_LANDINGPAD_OLD: {
       if (BitCode == bitc::FUNC_CODE_INST_LANDINGPAD) {
         if (Record.size() < 3)
           return error("Invalid record");
       } else {
         assert(BitCode == bitc::FUNC_CODE_INST_LANDINGPAD_OLD);
         if (Record.size() < 4)
           return error("Invalid record");
       }
-      Type *Ty = getTypeByID(Record[Idx++]);
+      FullTy = getFullyStructuredTypeByID(Record[Idx++]);
+      Type *Ty = flattenPointerTypes(FullTy);
       if (!Ty)
         return error("Invalid record");
       if (BitCode == bitc::FUNC_CODE_INST_LANDINGPAD_OLD) {
         Value *PersFn = nullptr;
         if (getValueTypePair(Record, Idx, NextValueNo, PersFn))
           return error("Invalid record");
 
         if (!F->hasPersonalityFn())
case bitc::FUNC_CODE_INST_ALLOCA: { // ALLOCA: [instty, opty, op, align]
       uint64_t AlignRecord = Record[3];
       const uint64_t InAllocaMask = uint64_t(1) << 5;
       const uint64_t ExplicitTypeMask = uint64_t(1) << 6;
       const uint64_t SwiftErrorMask = uint64_t(1) << 7;
       const uint64_t FlagMask = InAllocaMask | ExplicitTypeMask |
                                 SwiftErrorMask;
       bool InAlloca = AlignRecord & InAllocaMask;
       bool SwiftError = AlignRecord & SwiftErrorMask;
-      Type *Ty = getTypeByID(Record[0]);
+      FullTy = getFullyStructuredTypeByID(Record[0]);
+      Type *Ty = flattenPointerTypes(FullTy);
       if ((AlignRecord & ExplicitTypeMask) == 0) {
         auto *PTy = dyn_cast_or_null<PointerType>(Ty);
         if (!PTy)
           return error("Old-style alloca with a non-pointer type");
-        Ty = PTy->getElementType();
+        std::tie(FullTy, Ty) = getPointerElementTypes(FullTy);
       }
       Type *OpTy = getTypeByID(Record[1]);
       Value *Size = getFnValueByID(Record[2], OpTy);
       unsigned Align;
       if (Error Err = parseAlignmentValue(AlignRecord & ~FlagMask, Align)) {
         return Err;
       }
       if (!Ty || !Size)
         return error("Invalid record");
 
       // FIXME: Make this an optional field.
       const DataLayout &DL = TheModule->getDataLayout();
       unsigned AS = DL.getAllocaAddrSpace();
 
       AllocaInst *AI = new AllocaInst(Ty, AS, Size, Align);
       AI->setUsedWithInAlloca(InAlloca);
       AI->setSwiftError(SwiftError);
       I = AI;
+      FullTy = PointerType::get(FullTy, AS);
       InstructionList.push_back(I);
       break;
     }
     case bitc::FUNC_CODE_INST_LOAD: { // LOAD: [opty, op, align, vol]
       unsigned OpNum = 0;
       Value *Op;
-      if (getValueTypePair(Record, OpNum, NextValueNo, Op) ||
+      if (getValueTypePair(Record, OpNum, NextValueNo, Op, &FullTy) ||
           (OpNum + 2 != Record.size() && OpNum + 3 != Record.size()))
         return error("Invalid record");
 
+      if (!isa<PointerType>(Op->getType()))
+        return error("Load operand is not a pointer type");
+
       Type *Ty = nullptr;
-      if (OpNum + 3 == Record.size())
-        Ty = getTypeByID(Record[OpNum++]);
+      if (OpNum + 3 == Record.size()) {
+        FullTy = getFullyStructuredTypeByID(Record[OpNum++]);
+        Ty = flattenPointerTypes(FullTy);
+      } else
+        std::tie(FullTy, Ty) = getPointerElementTypes(FullTy);
+
       if (Error Err = typeCheckLoadStoreInst(Ty, Op->getType()))
         return Err;
-      if (!Ty)
-        Ty = cast<PointerType>(Op->getType())->getElementType();
 
       unsigned Align;
       if (Error Err = parseAlignmentValue(Record[OpNum], Align))
         return Err;
       I = new LoadInst(Ty, Op, "", Record[OpNum + 1], Align);
-
       InstructionList.push_back(I);
       break;
     }
     case bitc::FUNC_CODE_INST_LOADATOMIC: {
        // LOADATOMIC: [opty, op, align, vol, ordering, ssid]
       unsigned OpNum = 0;
       Value *Op;
-      if (getValueTypePair(Record, OpNum, NextValueNo, Op) ||
+      if (getValueTypePair(Record, OpNum, NextValueNo, Op, &FullTy) ||
           (OpNum + 4 != Record.size() && OpNum + 5 != Record.size()))
         return error("Invalid record");
 
+      if (!isa<PointerType>(Op->getType()))
+        return error("Load operand is not a pointer type");
+
       Type *Ty = nullptr;
-      if (OpNum + 5 == Record.size())
-        Ty = getTypeByID(Record[OpNum++]);
+      if (OpNum + 5 == Record.size()) {
+        FullTy = getFullyStructuredTypeByID(Record[OpNum++]);
+        Ty = flattenPointerTypes(FullTy);
+      } else
+        std::tie(FullTy, Ty) = getPointerElementTypes(FullTy);
+
       if (Error Err = typeCheckLoadStoreInst(Ty, Op->getType()))
         return Err;
-      if (!Ty)
-        Ty = cast<PointerType>(Op->getType())->getElementType();
 
       AtomicOrdering Ordering = getDecodedOrdering(Record[OpNum + 2]);
       if (Ordering == AtomicOrdering::NotAtomic ||
           Ordering == AtomicOrdering::Release ||
           Ordering == AtomicOrdering::AcquireRelease)
         return error("Invalid record");
       if (Ordering != AtomicOrdering::NotAtomic && Record[OpNum] == 0)
         return error("Invalid record");
       SyncScope::ID SSID = getDecodedSyncScopeID(Record[OpNum + 3]);
 
       unsigned Align;
       if (Error Err = parseAlignmentValue(Record[OpNum], Align))
         return Err;
       I = new LoadInst(Ty, Op, "", Record[OpNum + 1], Align, Ordering, SSID);
-
       InstructionList.push_back(I);
       break;
     }
     case bitc::FUNC_CODE_INST_STORE:
     case bitc::FUNC_CODE_INST_STORE_OLD: { // STORE2:[ptrty, ptr, val, align, vol]
       unsigned OpNum = 0;
       Value *Val, *Ptr;
-      if (getValueTypePair(Record, OpNum, NextValueNo, Ptr) ||
+      Type *FullTy;
+      if (getValueTypePair(Record, OpNum, NextValueNo, Ptr, &FullTy) ||
           (BitCode == bitc::FUNC_CODE_INST_STORE
                ? getValueTypePair(Record, OpNum, NextValueNo, Val)
                : popValue(Record, OpNum, NextValueNo,
-                          cast<PointerType>(Ptr->getType())->getElementType(),
-                          Val)) ||
+                          getPointerElementFlatType(FullTy), Val)) ||
           OpNum + 2 != Record.size())
         return error("Invalid record");
 
       if (Error Err = typeCheckLoadStoreInst(Val->getType(), Ptr->getType()))
         return Err;
       unsigned Align;
       if (Error Err = parseAlignmentValue(Record[OpNum], Align))
         return Err;
       I = new StoreInst(Val, Ptr, Record[OpNum+1], Align);
       InstructionList.push_back(I);
       break;
     }
     case bitc::FUNC_CODE_INST_STOREATOMIC:
     case bitc::FUNC_CODE_INST_STOREATOMIC_OLD: {
       // STOREATOMIC: [ptrty, ptr, val, align, vol, ordering, ssid]
       unsigned OpNum = 0;
       Value *Val, *Ptr;
-      if (getValueTypePair(Record, OpNum, NextValueNo, Ptr) ||
+      Type *FullTy;
+      if (getValueTypePair(Record, OpNum, NextValueNo, Ptr, &FullTy) ||
           !isa<PointerType>(Ptr->getType()) ||
           (BitCode == bitc::FUNC_CODE_INST_STOREATOMIC
                ? getValueTypePair(Record, OpNum, NextValueNo, Val)
                : popValue(Record, OpNum, NextValueNo,
-                          cast<PointerType>(Ptr->getType())->getElementType(),
-                          Val)) ||
+                          getPointerElementFlatType(FullTy), Val)) ||
           OpNum + 4 != Record.size())
         return error("Invalid record");
 
       if (Error Err = typeCheckLoadStoreInst(Val->getType(), Ptr->getType()))
         return Err;
       AtomicOrdering Ordering = getDecodedOrdering(Record[OpNum + 2]);
       if (Ordering == AtomicOrdering::NotAtomic ||
           Ordering == AtomicOrdering::Acquire ||
case bitc::FUNC_CODE_INST_STOREATOMIC_OLD: {
       break;
     }
     case bitc::FUNC_CODE_INST_CMPXCHG_OLD:
     case bitc::FUNC_CODE_INST_CMPXCHG: {
       // CMPXCHG:[ptrty, ptr, cmp, new, vol, successordering, ssid,
       //          failureordering?, isweak?]
       unsigned OpNum = 0;
       Value *Ptr, *Cmp, *New;
-      if (getValueTypePair(Record, OpNum, NextValueNo, Ptr) ||
-          (BitCode == bitc::FUNC_CODE_INST_CMPXCHG
-               ? getValueTypePair(Record, OpNum, NextValueNo, Cmp)
-               : popValue(Record, OpNum, NextValueNo,
-                          cast<PointerType>(Ptr->getType())->getElementType(),
-                          Cmp)) ||
-          popValue(Record, OpNum, NextValueNo, Cmp->getType(), New) ||
+      if (getValueTypePair(Record, OpNum, NextValueNo, Ptr, &FullTy))
+        return error("Invalid record");
+
+      if (!isa<PointerType>(Ptr->getType()))
+        return error("Cmpxchg operand is not a pointer type");
+
+      if (BitCode == bitc::FUNC_CODE_INST_CMPXCHG) {
+        if (getValueTypePair(Record, OpNum, NextValueNo, Cmp, &FullTy))
+          return error("Invalid record");
+      } else if (popValue(Record, OpNum, NextValueNo,
+                          getPointerElementFlatType(FullTy), Cmp))
+        return error("Invalid record");
+      else
+        FullTy = cast<PointerType>(FullTy)->getElementType();
+
+      if (popValue(Record, OpNum, NextValueNo, Cmp->getType(), New) ||
           Record.size() < OpNum + 3 || Record.size() > OpNum + 5)
         return error("Invalid record");
+
       AtomicOrdering SuccessOrdering = getDecodedOrdering(Record[OpNum + 1]);
       if (SuccessOrdering == AtomicOrdering::NotAtomic ||
           SuccessOrdering == AtomicOrdering::Unordered)
         return error("Invalid record");
       SyncScope::ID SSID = getDecodedSyncScopeID(Record[OpNum + 2]);
 
       if (Error Err = typeCheckLoadStoreInst(Cmp->getType(), Ptr->getType()))
         return Err;
       AtomicOrdering FailureOrdering;
       if (Record.size() < 7)
         FailureOrdering =
             AtomicCmpXchgInst::getStrongestFailureOrdering(SuccessOrdering);
       else
         FailureOrdering = getDecodedOrdering(Record[OpNum + 3]);
 
       I = new AtomicCmpXchgInst(Ptr, Cmp, New, SuccessOrdering, FailureOrdering,
                                 SSID);
+      FullTy = StructType::get(Context, {FullTy, Type::getInt1Ty(Context)});
       cast<AtomicCmpXchgInst>(I)->setVolatile(Record[OpNum]);
 
       if (Record.size() < 8) {
         // Before weak cmpxchgs existed, the instruction simply returned the
         // value loaded from memory, so bitcode files from that era will be
         // expecting the first component of a modern cmpxchg.
         CurBB->getInstList().push_back(I);
         I = ExtractValueInst::Create(I, 0);
+        FullTy = cast<StructType>(FullTy)->getElementType(0);
       } else {
         cast<AtomicCmpXchgInst>(I)->setWeak(Record[OpNum+4]);
       }
 
       InstructionList.push_back(I);
       break;
     }
     case bitc::FUNC_CODE_INST_ATOMICRMW: {
       // ATOMICRMW:[ptrty, ptr, val, op, vol, ordering, ssid]
       unsigned OpNum = 0;
       Value *Ptr, *Val;
-      if (getValueTypePair(Record, OpNum, NextValueNo, Ptr) ||
+      if (getValueTypePair(Record, OpNum, NextValueNo, Ptr, &FullTy) ||
           !isa<PointerType>(Ptr->getType()) ||
           popValue(Record, OpNum, NextValueNo,
-                    cast<PointerType>(Ptr->getType())->getElementType(), Val) ||
-          OpNum+4 != Record.size())
+                   getPointerElementFlatType(FullTy), Val) ||
+          OpNum + 4 != Record.size())
         return error("Invalid record");
       AtomicRMWInst::BinOp Operation = getDecodedRMWOperation(Record[OpNum]);
       if (Operation < AtomicRMWInst::FIRST_BINOP ||
           Operation > AtomicRMWInst::LAST_BINOP)
         return error("Invalid record");
       AtomicOrdering Ordering = getDecodedOrdering(Record[OpNum + 2]);
       if (Ordering == AtomicOrdering::NotAtomic ||
           Ordering == AtomicOrdering::Unordered)
         return error("Invalid record");
       SyncScope::ID SSID = getDecodedSyncScopeID(Record[OpNum + 3]);
       I = new AtomicRMWInst(Operation, Ptr, Val, Ordering, SSID);
+      FullTy = getPointerElementFlatType(FullTy);
       cast<AtomicRMWInst>(I)->setVolatile(Record[OpNum+1]);
       InstructionList.push_back(I);
       break;
     }
     case bitc::FUNC_CODE_INST_FENCE: { // FENCE:[ordering, ssid]
       if (2 != Record.size())
         return error("Invalid record");
       AtomicOrdering Ordering = getDecodedOrdering(Record[0]);
case bitc::FUNC_CODE_INST_CALL: {
       FastMathFlags FMF;
       if ((CCInfo >> bitc::CALL_FMF) & 1) {
         FMF = getDecodedFastMathFlags(Record[OpNum++]);
         if (!FMF.any())
           return error("Fast math flags indicator set for call with no FMF");
       }
 
       FunctionType *FTy = nullptr;
-      if (CCInfo >> bitc::CALL_EXPLICIT_TYPE & 1 &&
-          !(FTy = dyn_cast<FunctionType>(getTypeByID(Record[OpNum++]))))
+      FunctionType *FullFTy = nullptr;
+      if ((CCInfo >> bitc::CALL_EXPLICIT_TYPE) & 1) {
+        FullFTy =
+            dyn_cast<FunctionType>(getFullyStructuredTypeByID(Record[OpNum++]));
+        if (!FullFTy)
           return error("Explicit call type is not a function type");
+        FTy = cast<FunctionType>(flattenPointerTypes(FullFTy));
+      }
 
       Value *Callee;
-      if (getValueTypePair(Record, OpNum, NextValueNo, Callee))
+      if (getValueTypePair(Record, OpNum, NextValueNo, Callee, &FullTy))
         return error("Invalid record");
 
       PointerType *OpTy = dyn_cast<PointerType>(Callee->getType());
       if (!OpTy)
         return error("Callee is not a pointer type");
       if (!FTy) {
-        FTy = dyn_cast<FunctionType>(OpTy->getElementType());
-        if (!FTy)
+        FullFTy =
+            dyn_cast<FunctionType>(cast<PointerType>(FullTy)->getElementType());
+        if (!FullFTy)
           return error("Callee is not of pointer to function type");
-      } else if (OpTy->getElementType() != FTy)
+        FTy = cast<FunctionType>(flattenPointerTypes(FullFTy));
+      } else if (getPointerElementFlatType(FullTy) != FTy)
         return error("Explicit call type does not match pointee type of "
                      "callee operand");
       if (Record.size() < FTy->getNumParams() + OpNum)
         return error("Insufficient operands to call");
 
       SmallVector<Value*, 16> Args;
+      SmallVector<Type*, 16> ArgsFullTys;
       // Read the fixed params.
       for (unsigned i = 0, e = FTy->getNumParams(); i != e; ++i, ++OpNum) {
         if (FTy->getParamType(i)->isLabelTy())
           Args.push_back(getBasicBlock(Record[OpNum]));
         else
           Args.push_back(getValue(Record, OpNum, NextValueNo,
                                   FTy->getParamType(i)));
+        ArgsFullTys.push_back(FullFTy->getParamType(i));
         if (!Args.back())
           return error("Invalid record");
       }
 
       // Read type/value pairs for varargs params.
       if (!FTy->isVarArg()) {
         if (OpNum != Record.size())
           return error("Invalid record");
       } else {
         while (OpNum != Record.size()) {
           Value *Op;
-          if (getValueTypePair(Record, OpNum, NextValueNo, Op))
+          Type *FullTy;
+          if (getValueTypePair(Record, OpNum, NextValueNo, Op, &FullTy))
             return error("Invalid record");
           Args.push_back(Op);
+          ArgsFullTys.push_back(FullTy);
         }
       }
 
       I = CallInst::Create(FTy, Callee, Args, OperandBundles);
+      FullTy = FullFTy->getReturnType();
       OperandBundles.clear();
       InstructionList.push_back(I);
       cast<CallInst>(I)->setCallingConv(
           static_cast<CallingConv::ID>((0x7ff & CCInfo) >> bitc::CALL_CCONV));
       CallInst::TailCallKind TCK = CallInst::TCK_None;
       if (CCInfo & 1 << bitc::CALL_TAIL)
         TCK = CallInst::TCK_Tail;
       if (CCInfo & (1 << bitc::CALL_MUSTTAIL))
         TCK = CallInst::TCK_MustTail;
       if (CCInfo & (1 << bitc::CALL_NOTAIL))
         TCK = CallInst::TCK_NoTail;
       cast<CallInst>(I)->setTailCallKind(TCK);
       cast<CallInst>(I)->setAttributes(PAL);
-      propagateByValTypes(cast<CallBase>(I));
+      propagateByValTypes(cast<CallBase>(I), ArgsFullTys);
       if (FMF.any()) {
         if (!isa<FPMathOperator>(I))
           return error("Fast-math-flags specified for call without "
                        "floating-point scalar or vector return type");
         I->setFastMathFlags(FMF);
       }
       break;
     }
     case bitc::FUNC_CODE_INST_VAARG: { // VAARG: [valistty, valist, instty]
       if (Record.size() < 3)
         return error("Invalid record");
       Type *OpTy = getTypeByID(Record[0]);
       Value *Op = getValue(Record, 1, NextValueNo, OpTy);
-      Type *ResTy = getTypeByID(Record[2]);
+      FullTy = getFullyStructuredTypeByID(Record[2]);
+      Type *ResTy = flattenPointerTypes(FullTy);
       if (!OpTy || !Op || !ResTy)
         return error("Invalid record");
       I = new VAArgInst(Op, ResTy);
       InstructionList.push_back(I);
       break;
     }
 
     case bitc::FUNC_CODE_OPERAND_BUNDLE: {

 
     // If this was a terminator instruction, move to the next block.
     if (I->isTerminator()) {
       ++CurBBNo;
       CurBB = CurBBNo < FunctionBBs.size() ? FunctionBBs[CurBBNo] : nullptr;
     }
 
     // Non-void values get registered in the value table for future use.
-    if (I && !I->getType()->isVoidTy())
-      ValueList.assignValue(I, NextValueNo++);
+    if (I && !I->getType()->isVoidTy()) {
+      if (!FullTy) {
+        FullTy = I->getType();
+        assert(
+            !FullTy->isPointerTy() && !isa<StructType>(FullTy) &&
+            !isa<ArrayType>(FullTy) &&
+            (!isa<VectorType>(FullTy) ||
+             FullTy->getVectorElementType()->isFloatingPointTy() ||
+             FullTy->getVectorElementType()->isIntegerTy()) &&
+            "Structured types must be assigned with corresponding non-opaque "
+            "pointer type");
+      }
+
+      assert(I->getType() == flattenPointerTypes(FullTy) &&
+             "Incorrect fully structured type provided for Instruction");
+      ValueList.assignValue(I, NextValueNo++, FullTy);
+    }
   }
 
 OutOfRecordLoop:
 
   if (!OperandBundles.empty())
     return error("Operand bundles found with no consumer");
 
   // Check the function list for unresolved values.

llvm/lib/Bitcode/Reader/ValueList.h

 class Constant;
 class LLVMContext;
 class Type;
 class Value;
 
 class BitcodeReaderValueList {
   std::vector<WeakTrackingVH> ValuePtrs;
 
+  /// Struct containing fully-specified copies of the type of each
+  /// value. When pointers are opaque, this will be contain non-opaque
+  /// variants so that restructuring instructions can determine their
+  /// type correctly even if being loaded from old bitcode where some
+  /// types are implicit.
+  std::vector<Type *> FullTypes;
+
   /// As we resolve forward-referenced constants, we add information about them
   /// to this vector.  This allows us to resolve them in bulk instead of
   /// resolving each reference at a time.  See the code in
   /// ResolveConstantForwardRefs for more information about this.
   ///
   /// The key of this vector is the placeholder constant, the value is the slot
   /// number that holds the resolved value.
   using ResolveConstantsTy = std::vector<std::pair<Constant *, unsigned>>;
   ResolveConstantsTy ResolveConstants;
   LLVMContext &Context;
 
 public:
   BitcodeReaderValueList(LLVMContext &C) : Context(C) {}
 
   ~BitcodeReaderValueList() {
     assert(ResolveConstants.empty() && "Constants not resolved?");
   }
 
   // vector compatibility methods
   unsigned size() const { return ValuePtrs.size(); }
-  void resize(unsigned N) { ValuePtrs.resize(N); }
-  void push_back(Value *V) { ValuePtrs.emplace_back(V); }
+  void resize(unsigned N) {
+    ValuePtrs.resize(N);
+    FullTypes.resize(N);
+  }
+  void push_back(Value *V, Type *Ty) {
+    ValuePtrs.emplace_back(V);
+    FullTypes.emplace_back(Ty);
+  }
 
   void clear() {
     assert(ResolveConstants.empty() && "Constants not resolved?");
     ValuePtrs.clear();
+    FullTypes.clear();
   }
 
   Value *operator[](unsigned i) const {
     assert(i < ValuePtrs.size());
     return ValuePtrs[i];
   }
 
   Value *back() const { return ValuePtrs.back(); }
-  void pop_back() { ValuePtrs.pop_back(); }
+  void pop_back() {
+    ValuePtrs.pop_back();
+    FullTypes.pop_back();
+  }
   bool empty() const { return ValuePtrs.empty(); }
 
   void shrinkTo(unsigned N) {
     assert(N <= size() && "Invalid shrinkTo request!");
     ValuePtrs.resize(N);
+    FullTypes.resize(N);
   }
 
   Constant *getConstantFwdRef(unsigned Idx, Type *Ty);
-  Value *getValueFwdRef(unsigned Idx, Type *Ty);
+  Value *getValueFwdRef(unsigned Idx, Type *Ty, Type **FullTy = nullptr);
 
-  void assignValue(Value *V, unsigned Idx);
+  void assignValue(Value *V, unsigned Idx, Type *FullTy);
 
   /// Once all constants are read, this method bulk resolves any forward
   /// references.
   void resolveConstantForwardRefs();
 };
 
 } // end namespace llvm
 
 #endif // LLVM_LIB_BITCODE_READER_VALUELIST_H

llvm/lib/Bitcode/Reader/ValueList.cpp

 // FIXME: can we inherit this from ConstantExpr?
 template <>
 struct OperandTraits<ConstantPlaceHolder>
     : public FixedNumOperandTraits<ConstantPlaceHolder, 1> {};
 DEFINE_TRANSPARENT_OPERAND_ACCESSORS(ConstantPlaceHolder, Value)
 
 } // end namespace llvm
 
-void BitcodeReaderValueList::assignValue(Value *V, unsigned Idx) {
+void BitcodeReaderValueList::assignValue(Value *V, unsigned Idx, Type *FullTy) {
   if (Idx == size()) {
-    push_back(V);
+    push_back(V, FullTy);
     return;
   }
 
   if (Idx >= size())
     resize(Idx + 1);
 
+  assert(FullTypes[Idx] == nullptr || FullTypes[Idx] == FullTy);
+  FullTypes[Idx] = FullTy;
+
   WeakTrackingVH &OldV = ValuePtrs[Idx];
   if (!OldV) {
     OldV = V;
     return;
   }
 
   // Handle constants and non-constants (e.g. instrs) differently for
   // efficiency.
Constant *BitcodeReaderValueList::getConstantFwdRef(unsigned Idx, Type *Ty) {
   }
 
   // Create and return a placeholder, which will later be RAUW'd.
   Constant *C = new ConstantPlaceHolder(Ty, Context);
   ValuePtrs[Idx] = C;
   return C;
 }
 
-Value *BitcodeReaderValueList::getValueFwdRef(unsigned Idx, Type *Ty) {
+Value *BitcodeReaderValueList::getValueFwdRef(unsigned Idx, Type *Ty,
+                                              Type **FullTy) {
   // Bail out for a clearly invalid value. This would make us call resize(0)
   if (Idx == std::numeric_limits<unsigned>::max())
     return nullptr;
 
   if (Idx >= size())
     resize(Idx + 1);
 
   if (Value *V = ValuePtrs[Idx]) {
     // If the types don't match, it's invalid.
     if (Ty && Ty != V->getType())
       return nullptr;
+    if (FullTy)
+      *FullTy = FullTypes[Idx];
     return V;
   }
 
   // No type specified, must be invalid reference.
   if (!Ty)
     return nullptr;
 
   // Create and return a placeholder, which will later be RAUW'd.

llvm/test/Bitcode/invalid.test

 RUN: not llvm-dis -disable-output %p/Inputs/invalid-too-big-fwdref.bc 2>&1 | \
 RUN:   FileCheck --check-prefix=HUGE-FWDREF %s
 
 HUGE-FWDREF: Invalid record
 
 RUN: not llvm-dis -disable-output %p/Inputs/invalid-load-pointer-type.bc 2>&1 | \
 RUN:   FileCheck --check-prefix=LOAD-BAD-TYPE %s
 
-LOAD-BAD-TYPE: Load/Store operand is not a pointer type
+LOAD-BAD-TYPE: Load operand is not a pointer type
 
 RUN: not llvm-dis -disable-output %p/Inputs/invalid-GCTable-overflow.bc 2>&1 | \
 RUN:   FileCheck --check-prefix=GCTABLE-OFLOW %s
 
 GCTABLE-OFLOW: Invalid ID
 
 RUN: not llvm-dis -disable-output %p/Inputs/invalid-insert-0-indices.bc 2>&1 | \
 RUN:   FileCheck --check-prefix=INSERT-0-IDXS %s