Support converting pointers from opaque to typed

[beanz]

Summary

Using the pointer type analysis we can re-constitute typed pointers and
populate the correct types in the bitcasts throughout the IR.

This doesn't yet handle all cases, but this should be illustrative as to the dirction and feasability of
the solution.

[lebedev.ri]

I'm guessing this isn't actually meant for upstreaming, is it?

[beanz]

In D122270#3402106, @lebedev.ri wrote:

I'm guessing this isn't actually meant for upstreaming, is it?

It very much is. This is part of the work required to support DXIL generation. See: https://discourse.llvm.org/t/rfc-adding-hlsl-and-directx-support-to-clang-llvm/60783/

[lebedev.ri]

In D122270#3402238, @beanz wrote:

In D122270#3402106, @lebedev.ri wrote:

I'm guessing this isn't actually meant for upstreaming, is it?

It very much is. This is part of the work required to support DXIL generation. See: https://discourse.llvm.org/t/rfc-adding-hlsl-and-directx-support-to-clang-llvm/60783/

Then i don't believe this design as present is acceptable for inclusion into llvm.

[beanz]

In D122270#3402244, @lebedev.ri wrote:

Then i don't believe this design as present is acceptable for inclusion into llvm.

Please elaborate.

[beanz]

A few notes about this PR.

For those not familiar with the bitcode writing infrastructure, the ValueEnumerator assigns stable IDs to IR constructs (llvm::Values, llvm::Types...) so they can refer to each other. It is not directly involved in writing bitcode, and the general approach in the ValueEnumerator has been unchanged for a very long time.

I'm trying to balance two conflicting factors with the work to support DXIL:
(1) Keeping the DirectX backend small and easy to maintain by avoiding unnecessary code duplication
-and-
(2) Keeping the DXIL writer contained in the backend

This patch adds a small override to the ValueEnumerator to insert an i8* type at the beginning of the type list. Having i8* at the beginning of the list is required to satisfy TypeList ordering constraints that the 3.7 bitcode reader (and the modern one too) requires.

[beanz]

Updating to use dxil::TypedPointerType, and handle FunctionTypes containing opaque pointers.

[aeubanks]

In D122270#3402489, @beanz wrote:

A few notes about this PR.

For those not familiar with the bitcode writing infrastructure, the ValueEnumerator assigns stable IDs to IR constructs (llvm::Values, llvm::Types...) so they can refer to each other. It is not directly involved in writing bitcode, and the general approach in the ValueEnumerator has been unchanged for a very long time.

I'm trying to balance two conflicting factors with the work to support DXIL:
(1) Keeping the DirectX backend small and easy to maintain by avoiding unnecessary code duplication
-and-
(2) Keeping the DXIL writer contained in the backend

This patch adds a small override to the ValueEnumerator to insert an i8* type at the beginning of the type list. Having i8* at the beginning of the list is required to satisfy TypeList ordering constraints that the 3.7 bitcode reader (and the modern one too) requires.

I would consider forking the bitcode writer and its dependencies like ValueEnumerator rather than trying to share code. Having to support 3.7 bitcode features in the "official" bitcode writer doesn't seem appealing, however small the features may be. I see 3.7 bitcode as a completely new backend rather than a slight variation on LLVM head IR. The bitcode reader is already quite complicated due to maintaining backward compatibility, I would prefer if that didn't happen with the bitcode writer.

[beanz]

In D122270#3406636, @aeubanks wrote:

I would consider forking the bitcode writer and its dependencies like ValueEnumerator rather than trying to share code. Having to support 3.7 bitcode features in the "official" bitcode writer doesn't seem appealing, however small the features may be. I see 3.7 bitcode as a completely new backend rather than a slight variation on LLVM head IR. The bitcode reader is already quite complicated due to maintaining backward compatibility, I would prefer if that didn't happen with the bitcode writer.

That is a reasonable approach. Worth noting, the ValueEnumerator really just maps IR constructs to stable IDs, it is used by the bitcode writer, but other than a few cases of unclean abstractions, has no real binding to the bitcode format.

My patch does completely fork the bitcode writer, I'm just trying to share the ValueEnumerator and bitcode writer pass at the moment. I can also fork both of those too.

[MaskRay]

Sound good to fork BitcodeWriter for DXIL. When the masm-style assembler was added, there was also a decision that it should be forked to llvm/lib/MC/MCParser/MasmParser.cpp to avoid complexity to the GNU assembler/Darwin assembler style AsmParser.cpp

[nhaehnle]

This approach looks good to me.

Depending on bitcast instructions inserted by the DXILPrepare pass feels slightly iffy. It's an assumption about the IR that is not covered by the standard "IR contract". Then again, it is quite common for the existing backends to have such assumptions, e.g. around legality in GlobalISel. So I think this is acceptable.

Making ValueEnumerator more generically useful makes a lot of sense to me, I really only have one concern about how that is done here, see the inline comment.

llvm/lib/Bitcode/Writer/ValueEnumerator.cpp

365 ↗

(On Diff #418050)

Can you explain why PrefixType is needed? It seems like a strange wart in the API.

If this is really necessary, I feel like a cleaner API for the ValueEnumerator itself would be to split the enumeration out of the constructor, so that it's used as (not sure where ShouldPreserveUseListOrder should go):

ValueEnumerator VE;
VE.EnumerateModule(M, ShouldPreserveUseListOrder);

You could then inject an EnumerateType in the DirectX backend. There may have to be a Finalize method that must be called exactly once.

I haven't thought about how that change would propagate through the BitCodeModuleWriter.

[beanz]

Rebasing patches on the updated patch stack.

[beanz]

@nhaehnle sorry, I realized this patch hadn't been rebased since the request to just fork the full bitcode writer.

The reason I add a "PrefixType" in the ValueEnumerator is because the ValueEnumerator's constructor does a module walk and registers types. Because Types can't be referenced out of order in the bitcode, I need to make sure to shove in the i8* type at the beginning.

My latest update to this patch keeps that behavior even though the full ValueEnumerator is forked, but with it forked I think there is a better solution I can implement to have it insert the pointer types in the right place, and move the PointerTypeMap into the ValueEnumerator (which probably makes more sense anyways).

I'll work on reworking this patch over the next day or two, but may not get an update uploaded until I get back from vacation on 4/25.

Thank you!

[beanz]

Rebasing and cleaning up code.

[kuhar]

llvm/lib/Target/DirectX/DXILWriter/CMakeLists.txt

2

Could we use target_include_directories instead or a global ones?

[beanz]

llvm/lib/Target/DirectX/DXILWriter/CMakeLists.txt

2

include_directories sets the directory property not the global one, so it only applies to this directory and any subdirectories. There's no reason we couldn't use the target-specific one, but there is no material difference.

[kuhar]

llvm/lib/Target/DirectX/DXILWriter/CMakeLists.txt

2

I see this similar to using global variables in C++. There's no difference *now* but in my experience cmake tends to grow by copy-paste-style development and in X years we may have N targets here all with the same include directories. As we port these to other builds systems like GN or blaze, having clear target_ includes/libraries/definitions makes life much easier.

[kuhar]

llvm/lib/Target/DirectX/DXILWriter/CMakeLists.txt

2

s/blaze/bazel

[beanz]

llvm/lib/Target/DirectX/DXILWriter/CMakeLists.txt

2

I'm a lot concerned about this.

On a technical level, target_include_directories is used in 3 places in LLVM (all except for one in modules not lists), however include_directories is used almost two dozen times many of them throughout the Target subdirectories. The convention is to use include_directories. Using directory-based build settings is fundamental to the LLVM build system and has been since the days of the old Makefile build. Anyone translating CMake to another build system needs to understand that and emulate CMake correctly otherwise this will all go off the rails.

I don't think you can draw a parallel between global variables in C++ and directory-scope properties in CMake. They're not the same thing, and our entire build system is built around directory-scoped behaviors which isn't uncommon or unique to CMake. Our old Makefile build had directory-based variable overrides too.

While this may seem like a small change (and should have no material impact), you're asking me to deviate from established conventions in the codebase and I don't believe your reasons justify it. Which leads me to my even bigger concern...

On a policy level, we absolutely should not be considering the impacts on other build systems. The GN and Bazel builds were allowed in-tree on the strict policy that they were optional and would not impact the development or usage of the CMake build. Had anyone suggested in integrating GN or Bazel that we would alter our CMake conventions or usage to accommodate the new build systems they likely wouldn't have been allowed in-tree. The single biggest concern and complaint about adding GN and Bazel was that the community must have _only_ one build system. As the guy who spent 18 months cleaning up CMake so that we could delete the Makefile builds, that's a really important feature.

If, as a policy, we want to move away from directory-based build settings we should have that larger discussion before pushing one patch in a different direction.

[kuhar]

llvm/lib/Target/DirectX/DXILWriter/CMakeLists.txt

2

I don't have a strong opinion on this, this was just a suggestion. Sorry if this wasn't initially clear from my comment, I'll try to be more explicit in the future.

target_include_directories is used in 3 places in LLVM (all except for one in modules not lists)
While this may seem like a small change (and should have no material impact), you're asking me to deviate from established conventions in the codebase and I don't believe your reasons justify it.

I counted 80, but that's probably still not much compared to the size of the monorepo.

➜ ~/llvm/llvm-project git:(main)                                                   
$ ag target_include -Gtxt | wc -l                                                  
80

compared to:

$ ag include_directories -Gtxt | grep -v target_ | wc -l
215

So unless I counted wrong, I wouldn't say that using target_include_directories would be a deviation from the existing practices in llvm-project.

On a policy level, we absolutely should not be considering the impacts on other build systems.
...

Other build systems don't consume cmake files in any way. What I wrote was a hand-wavy argument that being explicit with target_ makes it easier to understand what cmake targets consist of, and to support that claim, in my experience, using target_ makes it easier to port builds to other (more explicit) build systems.

If, as a policy, we want to move away from directory-based build settings we should have that larger discussion before pushing one patch in a different direction.

I'm not an expert on cmake by any means, but my impression was that this shift was largely agreed on and part of 'modern cmake': https://cliutils.gitlab.io/modern-cmake/chapters/basics.html#:~:text=Targets%20are%20your,an%20include%20directory%3A

On the high level, please don't spend too much time on this if you don't agree with this suggestion.

[pete]

This looks good to me.

Only thing I can think to add is a test for non-zero address spaces. So probably the gep/load/store tests you have already but an address space of 1 (or any address space that makes sense in DXIL, not sure if any are special).

[beanz]

Updating to handle addrspaces correctly.

[pete]

In D122270#3547371, @beanz wrote:

Updating to handle addrspaces correctly.

Thanks! Given that change, this all LGTM.

llvm/lib/Target/DirectX/DXILWriter/CMakeLists.txt

+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../)
+
 add_llvm_component_library(LLVMDXILBitWriter
   DXILBitcodeWriter.cpp
   DXILValueEnumerator.cpp
   DXILWriterPass.cpp
 
   DEPENDS
   intrinsics_gen
 
   LINK_COMPONENTS
   Bitwriter
   Core
   MC
   Object
   Support
   )

llvm/lib/Target/DirectX/DXILWriter/DXILBitcodeWriter.cpp

 //===- Bitcode/Writer/DXILBitcodeWriter.cpp - DXIL Bitcode Writer ---------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // Bitcode writer implementation.
 //
 //===----------------------------------------------------------------------===//
 
 #include "DXILBitcodeWriter.h"
 #include "DXILValueEnumerator.h"
+#include "PointerTypeAnalysis.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Bitcode/BitcodeCommon.h"
 #include "llvm/Bitcode/BitcodeReader.h"
 #include "llvm/Bitcode/LLVMBitCodes.h"
 #include "llvm/Bitstream/BitCodes.h"
 #include "llvm/Bitstream/BitstreamWriter.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
enum {
     FUNCTION_INST_BINOP_FLAGS_ABBREV,
     FUNCTION_INST_CAST_ABBREV,
     FUNCTION_INST_RET_VOID_ABBREV,
     FUNCTION_INST_RET_VAL_ABBREV,
     FUNCTION_INST_UNREACHABLE_ABBREV,
     FUNCTION_INST_GEP_ABBREV,
   };
 
+  // Cache some types
+  Type *I8Ty;
+  Type *I8PtrTy;
+
   /// The stream created and owned by the client.
   BitstreamWriter &Stream;
 
   StringTableBuilder &StrtabBuilder;
 
   /// The Module to write to bitcode.
   const Module &M;
 
class DXILBitcodeWriter {
   uint64_t VSTOffsetPlaceholder = 0;
 
   /// Pointer to the buffer allocated by caller for bitcode writing.
   const SmallVectorImpl<char> &Buffer;
 
   /// The start bit of the identification block.
   uint64_t BitcodeStartBit;
 
+  /// This maps values to their typed pointers
+  PointerTypeMap PointerMap;
+
 public:
   /// Constructs a ModuleBitcodeWriter object for the given Module,
   /// writing to the provided \p Buffer.
   DXILBitcodeWriter(const Module &M, SmallVectorImpl<char> &Buffer,
                     StringTableBuilder &StrtabBuilder, BitstreamWriter &Stream)
-      : Stream(Stream), StrtabBuilder(StrtabBuilder), M(M),
-        VE(M, true), Buffer(Buffer),
-        BitcodeStartBit(Stream.GetCurrentBitNo()) {
+      : I8Ty(Type::getInt8Ty(M.getContext())),
+        I8PtrTy(TypedPointerType::get(I8Ty, 0)),
+        Stream(Stream), StrtabBuilder(StrtabBuilder), M(M), VE(M, I8PtrTy),
+        Buffer(Buffer), BitcodeStartBit(Stream.GetCurrentBitNo()),
+        PointerMap(PointerTypeAnalysis::run(M))
+         {
     GlobalValueId = VE.getValues().size();
+    // Enumerate the typed pointers
+    for (auto El : PointerMap)
+      VE.EnumerateType(El.second);
   }
 
   /// Emit the current module to the bitstream.
   void write();
 
   static uint64_t getAttrKindEncoding(Attribute::AttrKind Kind);
   static void writeStringRecord(BitstreamWriter &Stream, unsigned Code,
                                 StringRef Str, unsigned AbbrevToUse);
private:
   void writeUseList(UseListOrder &&Order);
   void writeUseListBlock(const Function *F);
   void writeFunction(const Function &F);
   void writeBlockInfo();
 
   unsigned getEncodedSyncScopeID(SyncScope::ID SSID) { return unsigned(SSID); }
 
   unsigned getEncodedAlign(MaybeAlign Alignment) { return encode(Alignment); }
+
+  unsigned getTypeID(Type *T, const Value *V = nullptr);
+  unsigned getTypeID(Type *T, const Function *F);
 };
 
 } // namespace dxil
 } // namespace llvm
 
 using namespace llvm;
 using namespace llvm::dxil;
 
 ////////////////////////////////////////////////////////////////////////////////
 /// Begin dxil::BitcodeWriter Implementation
 ////////////////////////////////////////////////////////////////////////////////
 
-dxil::BitcodeWriter::BitcodeWriter(SmallVectorImpl<char> &Buffer, raw_fd_stream *FS)
+dxil::BitcodeWriter::BitcodeWriter(SmallVectorImpl<char> &Buffer,
+                                   raw_fd_stream *FS)
     : Buffer(Buffer), Stream(new BitstreamWriter(Buffer, FS, 512)) {
   // Emit the file header.
   Stream->Emit((unsigned)'B', 8);
   Stream->Emit((unsigned)'C', 8);
   Stream->Emit(0x0, 4);
   Stream->Emit(0xC, 4);
   Stream->Emit(0xE, 4);
   Stream->Emit(0xD, 4);
case Instruction::And:
     return bitc::BINOP_AND;
   case Instruction::Or:
     return bitc::BINOP_OR;
   case Instruction::Xor:
     return bitc::BINOP_XOR;
   }
 }
 
+unsigned DXILBitcodeWriter::getTypeID(Type *T, const Value *V) {
+  if (!T->isOpaquePointerTy())
+    return VE.getTypeID(T);
+  auto It = PointerMap.find(V);
+  if (It != PointerMap.end())
+    return VE.getTypeID(It->second);
+  return VE.getTypeID(I8PtrTy);
+}
+
+unsigned DXILBitcodeWriter::getTypeID(Type *T, const Function *F) {
+  auto It = PointerMap.find(F);
+  if (It != PointerMap.end())
+    return VE.getTypeID(It->second);
+  return VE.getTypeID(T);
+}
+
 unsigned DXILBitcodeWriter::getEncodedRMWOperation(AtomicRMWInst::BinOp Op) {
   switch (Op) {
   default:
     llvm_unreachable("Unknown RMW operation!");
   case AtomicRMWInst::Xchg:
     return bitc::RMW_XCHG;
   case AtomicRMWInst::Add:
     return bitc::RMW_ADD;

 }
 
 unsigned DXILBitcodeWriter::getEncodedLinkage(const GlobalValue &GV) {
   return getEncodedLinkage(GV.getLinkage());
 }
 
 unsigned DXILBitcodeWriter::getEncodedVisibility(const GlobalValue &GV) {
   switch (GV.getVisibility()) {
-  case GlobalValue::DefaultVisibility:   return 0;
-  case GlobalValue::HiddenVisibility:    return 1;
-  case GlobalValue::ProtectedVisibility: return 2;
+  case GlobalValue::DefaultVisibility:
+    return 0;
+  case GlobalValue::HiddenVisibility:
+    return 1;
+  case GlobalValue::ProtectedVisibility:
+    return 2;
   }
   llvm_unreachable("Invalid visibility");
 }
 
 unsigned DXILBitcodeWriter::getEncodedDLLStorageClass(const GlobalValue &GV) {
   switch (GV.getDLLStorageClass()) {
-  case GlobalValue::DefaultStorageClass:   return 0;
-  case GlobalValue::DLLImportStorageClass: return 1;
-  case GlobalValue::DLLExportStorageClass: return 2;
+  case GlobalValue::DefaultStorageClass:
+    return 0;
+  case GlobalValue::DLLImportStorageClass:
+    return 1;
+  case GlobalValue::DLLExportStorageClass:
+    return 2;
   }
   llvm_unreachable("Invalid DLL storage class");
 }
 
 unsigned DXILBitcodeWriter::getEncodedThreadLocalMode(const GlobalValue &GV) {
   switch (GV.getThreadLocalMode()) {
-    case GlobalVariable::NotThreadLocal:         return 0;
-    case GlobalVariable::GeneralDynamicTLSModel: return 1;
-    case GlobalVariable::LocalDynamicTLSModel:   return 2;
-    case GlobalVariable::InitialExecTLSModel:    return 3;
-    case GlobalVariable::LocalExecTLSModel:      return 4;
+  case GlobalVariable::NotThreadLocal:
+    return 0;
+  case GlobalVariable::GeneralDynamicTLSModel:
+    return 1;
+  case GlobalVariable::LocalDynamicTLSModel:
+    return 2;
+  case GlobalVariable::InitialExecTLSModel:
+    return 3;
+  case GlobalVariable::LocalExecTLSModel:
+    return 4;
   }
   llvm_unreachable("Invalid TLS model");
 }
 
 unsigned DXILBitcodeWriter::getEncodedComdatSelectionKind(const Comdat &C) {
   switch (C.getSelectionKind()) {
   case Comdat::Any:
     return bitc::COMDAT_SELECTION_KIND_ANY;
void DXILBitcodeWriter::writeTypeTable() {
   for (Type *T : TypeList) {
     int AbbrevToUse = 0;
     unsigned Code = 0;
 
     switch (T->getTypeID()) {
     case Type::BFloatTyID:
     case Type::X86_AMXTyID:
     case Type::TokenTyID:
-    case Type::DXILPointerTyID:
       llvm_unreachable("These should never be used!!!");
       break;
     case Type::VoidTyID:
       Code = bitc::TYPE_CODE_VOID;
       break;
     case Type::HalfTyID:
       Code = bitc::TYPE_CODE_HALF;
       break;
for (Type *T : TypeList) {
     case Type::X86_MMXTyID:
       Code = bitc::TYPE_CODE_X86_MMX;
       break;
     case Type::IntegerTyID:
       // INTEGER: [width]
       Code = bitc::TYPE_CODE_INTEGER;
       TypeVals.push_back(cast<IntegerType>(T)->getBitWidth());
       break;
+    case Type::DXILPointerTyID: {
+      TypedPointerType *PTy = cast<TypedPointerType>(T);
+      // POINTER: [pointee type, address space]
+      Code = bitc::TYPE_CODE_POINTER;
+      TypeVals.push_back(getTypeID(PTy->getElementType()));
+      unsigned AddressSpace = PTy->getAddressSpace();
+      TypeVals.push_back(AddressSpace);
+      if (AddressSpace == 0)
+        AbbrevToUse = PtrAbbrev;
+      break;
+    }
     case Type::PointerTyID: {
       PointerType *PTy = cast<PointerType>(T);
       // POINTER: [pointee type, address space]
       Code = bitc::TYPE_CODE_POINTER;
-      TypeVals.push_back(VE.getTypeID(PTy->getNonOpaquePointerElementType()));
+      // Emitting an empty struct type for the opaque pointer's type allows
+      // this to be order-independent. Non-struct types must be emitted in
+      // bitcode before they can be referenced.
+      if (PTy->isOpaquePointerTy()) {
+        TypeVals.push_back(false);
+        Code = bitc::TYPE_CODE_OPAQUE;
+        writeStringRecord(Stream, bitc::TYPE_CODE_STRUCT_NAME,
+                          "dxilOpaquePtrReservedName", StructNameAbbrev);
+      } else {
+        TypeVals.push_back(getTypeID(PTy->getNonOpaquePointerElementType()));
         unsigned AddressSpace = PTy->getAddressSpace();
         TypeVals.push_back(AddressSpace);
         if (AddressSpace == 0)
           AbbrevToUse = PtrAbbrev;
+      }
       break;
     }
     case Type::FunctionTyID: {
       FunctionType *FT = cast<FunctionType>(T);
       // FUNCTION: [isvararg, retty, paramty x N]
       Code = bitc::TYPE_CODE_FUNCTION;
       TypeVals.push_back(FT->isVarArg());
-      TypeVals.push_back(VE.getTypeID(FT->getReturnType()));
+      TypeVals.push_back(getTypeID(FT->getReturnType()));
       for (Type *PTy : FT->params())
-        TypeVals.push_back(VE.getTypeID(PTy));
+        TypeVals.push_back(getTypeID(PTy));
       AbbrevToUse = FunctionAbbrev;
       break;
     }
     case Type::StructTyID: {
       StructType *ST = cast<StructType>(T);
       // STRUCT: [ispacked, eltty x N]
       TypeVals.push_back(ST->isPacked());
       // Output all of the element types.
       for (Type *ElTy : ST->elements())
-        TypeVals.push_back(VE.getTypeID(ElTy));
+        TypeVals.push_back(getTypeID(ElTy));
 
       if (ST->isLiteral()) {
         Code = bitc::TYPE_CODE_STRUCT_ANON;
         AbbrevToUse = StructAnonAbbrev;
       } else {
         if (ST->isOpaque()) {
           Code = bitc::TYPE_CODE_OPAQUE;
         } else {
           Code = bitc::TYPE_CODE_STRUCT_NAMED;
           AbbrevToUse = StructNamedAbbrev;
         }
 
         // Emit the name if it is present.
         if (!ST->getName().empty())
           writeStringRecord(Stream, bitc::TYPE_CODE_STRUCT_NAME, ST->getName(),
                             StructNameAbbrev);
       }
       break;
     }
     case Type::ArrayTyID: {
       ArrayType *AT = cast<ArrayType>(T);
       // ARRAY: [numelts, eltty]
       Code = bitc::TYPE_CODE_ARRAY;
       TypeVals.push_back(AT->getNumElements());
-      TypeVals.push_back(VE.getTypeID(AT->getElementType()));
+      TypeVals.push_back(getTypeID(AT->getElementType()));
       AbbrevToUse = ArrayAbbrev;
       break;
     }
     case Type::FixedVectorTyID:
     case Type::ScalableVectorTyID: {
       VectorType *VT = cast<VectorType>(T);
       // VECTOR [numelts, eltty]
       Code = bitc::TYPE_CODE_VECTOR;
       TypeVals.push_back(VT->getElementCount().getKnownMinValue());
-      TypeVals.push_back(VE.getTypeID(VT->getElementType()));
+      TypeVals.push_back(getTypeID(VT->getElementType()));
       break;
     }
     }
 
     // Emit the finished record.
     Stream.EmitRecord(Code, TypeVals, AbbrevToUse);
     TypeVals.clear();
   }
writeStringRecord(Stream, bitc::MODULE_CODE_TRIPLE, M.getTargetTriple(),
                       0 /*TODO*/);
   const std::string &DL = M.getDataLayoutStr();
   if (!DL.empty())
     writeStringRecord(Stream, bitc::MODULE_CODE_DATALAYOUT, DL, 0 /*TODO*/);
   if (!M.getModuleInlineAsm().empty())
     writeStringRecord(Stream, bitc::MODULE_CODE_ASM, M.getModuleInlineAsm(),
                       0 /*TODO*/);
 
-  // Emit information about sections and GC, computing how many there are. Also
-  // compute the maximum alignment value.
+  // Emit information about sections and GC, computing how many there are.
+  // Also compute the maximum alignment value.
   std::map<std::string, unsigned> SectionMap;
   std::map<std::string, unsigned> GCMap;
   MaybeAlign MaxAlignment;
   unsigned MaxGlobalType = 0;
   const auto UpdateMaxAlignment = [&MaxAlignment](const MaybeAlign A) {
     if (A)
       MaxAlignment = !MaxAlignment ? *A : std::max(*MaxAlignment, *A);
   };
   for (const GlobalVariable &GV : M.globals()) {
     UpdateMaxAlignment(GV.getAlign());
-    MaxGlobalType = std::max(MaxGlobalType, VE.getTypeID(GV.getValueType()));
+    MaxGlobalType = std::max(MaxGlobalType, getTypeID(GV.getValueType(), &GV));
     if (GV.hasSection()) {
       // Give section names unique ID's.
       unsigned &Entry = SectionMap[std::string(GV.getSection())];
       if (!Entry) {
         writeStringRecord(Stream, bitc::MODULE_CODE_SECTIONNAME,
                           GV.getSection(), 0 /*TODO*/);
         Entry = SectionMap.size();
       }
if (F.hasGC()) {
         Entry = GCMap.size();
       }
     }
   }
 
   // Emit abbrev for globals, now that we know # sections and max alignment.
   unsigned SimpleGVarAbbrev = 0;
   if (!M.global_empty()) {
-    // Add an abbrev for common globals with no visibility or thread localness.
+    // Add an abbrev for common globals with no visibility or thread
+    // localness.
     auto Abbv = std::make_shared<BitCodeAbbrev>();
     Abbv->Add(BitCodeAbbrevOp(bitc::MODULE_CODE_GLOBALVAR));
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed,
                               Log2_32_Ceil(MaxGlobalType + 1)));
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6));   // AddrSpace << 2
                                                            //| explicitType << 1
                                                            //| constant
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6));   // Initializer.
void DXILBitcodeWriter::writeModuleInfo() {
   SmallVector<unsigned, 64> Vals;
   for (const GlobalVariable &GV : M.globals()) {
     unsigned AbbrevToUse = 0;
 
     // GLOBALVAR: [type, isconst, initid,
     //             linkage, alignment, section, visibility, threadlocal,
     //             unnamed_addr, externally_initialized, dllstorageclass,
     //             comdat]
-    Vals.push_back(VE.getTypeID(GV.getValueType()));
+    Vals.push_back(getTypeID(GV.getValueType(), &GV));
     Vals.push_back(
         GV.getType()->getAddressSpace() << 2 | 2 |
         (GV.isConstant() ? 1 : 0)); // HLSL Change - bitwise | was used with
                                     // unsigned int and bool
     Vals.push_back(
         GV.isDeclaration() ? 0 : (VE.getValueID(GV.getInitializer()) + 1));
     Vals.push_back(getEncodedLinkage(GV));
     Vals.push_back(getEncodedAlign(GV.getAlign()));
for (const GlobalVariable &GV : M.globals()) {
     Vals.clear();
   }
 
   // Emit the function proto information.
   for (const Function &F : M) {
     // FUNCTION:  [type, callingconv, isproto, linkage, paramattrs, alignment,
     //             section, visibility, gc, unnamed_addr, prologuedata,
     //             dllstorageclass, comdat, prefixdata, personalityfn]
-    Vals.push_back(VE.getTypeID(F.getFunctionType()));
+    Vals.push_back(getTypeID(F.getFunctionType(), &F));
     Vals.push_back(F.getCallingConv());
     Vals.push_back(F.isDeclaration());
     Vals.push_back(getEncodedLinkage(F));
     Vals.push_back(VE.getAttributeListID(F.getAttributes()));
     Vals.push_back(getEncodedAlign(F.getAlign()));
     Vals.push_back(F.hasSection() ? SectionMap[std::string(F.getSection())]
                                   : 0);
     Vals.push_back(getEncodedVisibility(F));
for (const Function &F : M) {
     unsigned AbbrevToUse = 0;
     Stream.EmitRecord(bitc::MODULE_CODE_FUNCTION, Vals, AbbrevToUse);
     Vals.clear();
   }
 
   // Emit the alias information.
   for (const GlobalAlias &A : M.aliases()) {
     // ALIAS: [alias type, aliasee val#, linkage, visibility]
-    Vals.push_back(VE.getTypeID(A.getValueType()));
+    Vals.push_back(getTypeID(A.getValueType(), &A));
     Vals.push_back(VE.getValueID(A.getAliasee()));
     Vals.push_back(getEncodedLinkage(A));
     Vals.push_back(getEncodedVisibility(A));
     Vals.push_back(getEncodedDLLStorageClass(A));
     Vals.push_back(getEncodedThreadLocalMode(A));
     Vals.push_back(A.getUnnamedAddr() != GlobalValue::UnnamedAddr::None);
     unsigned AbbrevToUse = 0;
     Stream.EmitRecord(bitc::MODULE_CODE_ALIAS_OLD, Vals, AbbrevToUse);
     Vals.clear();
   }
 }
 
 void DXILBitcodeWriter::writeValueAsMetadata(
     const ValueAsMetadata *MD, SmallVectorImpl<uint64_t> &Record) {
   // Mimic an MDNode with a value as one operand.
   Value *V = MD->getValue();
-  Record.push_back(VE.getTypeID(V->getType()));
+  Record.push_back(getTypeID(V->getType()));
   Record.push_back(VE.getValueID(V));
   Stream.EmitRecord(bitc::METADATA_VALUE, Record, 0);
   Record.clear();
 }
 
 void DXILBitcodeWriter::writeMDTuple(const MDTuple *N,
                                      SmallVectorImpl<uint64_t> &Record,
                                      unsigned Abbrev) {

 }
 
 void DXILBitcodeWriter::writeModuleMetadata() {
   if (!VE.hasMDs() && M.named_metadata_empty())
     return;
 
   Stream.EnterSubblock(bitc::METADATA_BLOCK_ID, 5);
 
-  // Emit all abbrevs upfront, so that the reader can jump in the middle of the
-  // block and load any metadata.
+  // Emit all abbrevs upfront, so that the reader can jump in the middle of
+  // the block and load any metadata.
   std::vector<unsigned> MDAbbrevs;
 
   MDAbbrevs.resize(MetadataAbbrev::LastPlusOne);
   MDAbbrevs[MetadataAbbrev::DILocationAbbrevID] = createDILocationAbbrev();
   MDAbbrevs[MetadataAbbrev::GenericDINodeAbbrevID] =
       createGenericDINodeAbbrev();
 
   unsigned NameAbbrev = 0;
void DXILBitcodeWriter::writeConstants(unsigned FirstVal, unsigned LastVal,
 
   const ValueEnumerator::ValueList &Vals = VE.getValues();
   Type *LastTy = nullptr;
   for (unsigned i = FirstVal; i != LastVal; ++i) {
     const Value *V = Vals[i].first;
     // If we need to switch types, do so now.
     if (V->getType() != LastTy) {
       LastTy = V->getType();
-      Record.push_back(VE.getTypeID(LastTy));
+      Record.push_back(getTypeID(LastTy));
       Stream.EmitRecord(bitc::CST_CODE_SETTYPE, Record,
                         CONSTANTS_SETTYPE_ABBREV);
       Record.clear();
     }
 
     if (const InlineAsm *IA = dyn_cast<InlineAsm>(V)) {
       Record.push_back(unsigned(IA->hasSideEffects()) |
                        unsigned(IA->isAlignStack()) << 1 |
if (C->isNullValue()) {
         Record.push_back(VE.getValueID(Op));
       AbbrevToUse = AggregateAbbrev;
     } else if (const ConstantExpr *CE = dyn_cast<ConstantExpr>(C)) {
       switch (CE->getOpcode()) {
       default:
         if (Instruction::isCast(CE->getOpcode())) {
           Code = bitc::CST_CODE_CE_CAST;
           Record.push_back(getEncodedCastOpcode(CE->getOpcode()));
-          Record.push_back(VE.getTypeID(C->getOperand(0)->getType()));
+          Record.push_back(getTypeID(C->getOperand(0)->getType()));
           Record.push_back(VE.getValueID(C->getOperand(0)));
           AbbrevToUse = CONSTANTS_CE_CAST_Abbrev;
         } else {
           assert(CE->getNumOperands() == 2 && "Unknown constant expr!");
           Code = bitc::CST_CODE_CE_BINOP;
           Record.push_back(getEncodedBinaryOpcode(CE->getOpcode()));
           Record.push_back(VE.getValueID(C->getOperand(0)));
           Record.push_back(VE.getValueID(C->getOperand(1)));
           uint64_t Flags = getOptimizationFlags(CE);
           if (Flags != 0)
             Record.push_back(Flags);
         }
         break;
       case Instruction::GetElementPtr: {
         Code = bitc::CST_CODE_CE_GEP;
         const auto *GO = cast<GEPOperator>(C);
         if (GO->isInBounds())
           Code = bitc::CST_CODE_CE_INBOUNDS_GEP;
-        Record.push_back(VE.getTypeID(GO->getSourceElementType()));
+        Record.push_back(getTypeID(GO->getSourceElementType()));
         for (unsigned i = 0, e = CE->getNumOperands(); i != e; ++i) {
-          Record.push_back(VE.getTypeID(C->getOperand(i)->getType()));
+          Record.push_back(getTypeID(C->getOperand(i)->getType()));
           Record.push_back(VE.getValueID(C->getOperand(i)));
         }
         break;
       }
       case Instruction::Select:
         Code = bitc::CST_CODE_CE_SELECT;
         Record.push_back(VE.getValueID(C->getOperand(0)));
         Record.push_back(VE.getValueID(C->getOperand(1)));
         Record.push_back(VE.getValueID(C->getOperand(2)));
         break;
       case Instruction::ExtractElement:
         Code = bitc::CST_CODE_CE_EXTRACTELT;
-        Record.push_back(VE.getTypeID(C->getOperand(0)->getType()));
+        Record.push_back(getTypeID(C->getOperand(0)->getType()));
         Record.push_back(VE.getValueID(C->getOperand(0)));
-        Record.push_back(VE.getTypeID(C->getOperand(1)->getType()));
+        Record.push_back(getTypeID(C->getOperand(1)->getType()));
         Record.push_back(VE.getValueID(C->getOperand(1)));
         break;
       case Instruction::InsertElement:
         Code = bitc::CST_CODE_CE_INSERTELT;
         Record.push_back(VE.getValueID(C->getOperand(0)));
         Record.push_back(VE.getValueID(C->getOperand(1)));
-        Record.push_back(VE.getTypeID(C->getOperand(2)->getType()));
+        Record.push_back(getTypeID(C->getOperand(2)->getType()));
         Record.push_back(VE.getValueID(C->getOperand(2)));
         break;
       case Instruction::ShuffleVector:
         // If the return type and argument types are the same, this is a
         // standard shufflevector instruction.  If the types are different,
         // then the shuffle is widening or truncating the input vectors, and
         // the argument type must also be encoded.
         if (C->getType() == C->getOperand(0)->getType()) {
           Code = bitc::CST_CODE_CE_SHUFFLEVEC;
         } else {
           Code = bitc::CST_CODE_CE_SHUFVEC_EX;
-          Record.push_back(VE.getTypeID(C->getOperand(0)->getType()));
+          Record.push_back(getTypeID(C->getOperand(0)->getType()));
         }
         Record.push_back(VE.getValueID(C->getOperand(0)));
         Record.push_back(VE.getValueID(C->getOperand(1)));
         Record.push_back(VE.getValueID(C->getOperand(2)));
         break;
       case Instruction::ICmp:
       case Instruction::FCmp:
         Code = bitc::CST_CODE_CE_CMP;
-        Record.push_back(VE.getTypeID(C->getOperand(0)->getType()));
+        Record.push_back(getTypeID(C->getOperand(0)->getType()));
         Record.push_back(VE.getValueID(C->getOperand(0)));
         Record.push_back(VE.getValueID(C->getOperand(1)));
         Record.push_back(CE->getPredicate());
         break;
       }
     } else if (const BlockAddress *BA = dyn_cast<BlockAddress>(C)) {
       Code = bitc::CST_CODE_BLOCKADDRESS;
-      Record.push_back(VE.getTypeID(BA->getFunction()->getType()));
+      Record.push_back(getTypeID(BA->getFunction()->getType()));
       Record.push_back(VE.getValueID(BA->getFunction()));
       Record.push_back(VE.getGlobalBasicBlockID(BA->getBasicBlock()));
     } else {
 #ifndef NDEBUG
       C->dump();
 #endif
       llvm_unreachable("Unknown constant!");
     }
     Stream.EmitRecord(Code, Record, AbbrevToUse);
     Record.clear();
   }
 
   Stream.ExitBlock();
 }
 
 void DXILBitcodeWriter::writeModuleConstants() {
   const ValueEnumerator::ValueList &Vals = VE.getValues();
 
-  // Find the first constant to emit, which is the first non-globalvalue value.
-  // We know globalvalues have been emitted by WriteModuleInfo.
+  // Find the first constant to emit, which is the first non-globalvalue
+  // value. We know globalvalues have been emitted by WriteModuleInfo.
   for (unsigned i = 0, e = Vals.size(); i != e; ++i) {
     if (!isa<GlobalValue>(Vals[i].first)) {
       writeConstants(i, Vals.size(), true);
       return;
     }
   }
 }
 
 /// pushValueAndType - The file has to encode both the value and type id for
 /// many values, because we need to know what type to create for forward
-/// references.  However, most operands are not forward references, so this type
-/// field is not needed.
+/// references.  However, most operands are not forward references, so this
+/// type field is not needed.
 ///
-/// This function adds V's value ID to Vals.  If the value ID is higher than the
-/// instruction ID, then it is a forward reference, and it also includes the
-/// type ID.  The value ID that is written is encoded relative to the InstID.
+/// This function adds V's value ID to Vals.  If the value ID is higher than
+/// the instruction ID, then it is a forward reference, and it also includes
+/// the type ID.  The value ID that is written is encoded relative to the
+/// InstID.
 bool DXILBitcodeWriter::pushValueAndType(const Value *V, unsigned InstID,
                                          SmallVectorImpl<unsigned> &Vals) {
   unsigned ValID = VE.getValueID(V);
   // Make encoding relative to the InstID.
   Vals.push_back(InstID - ValID);
   if (ValID >= InstID) {
-    Vals.push_back(VE.getTypeID(V->getType()));
+    Vals.push_back(getTypeID(V->getType(), V));
     return true;
   }
   return false;
 }
 
 /// pushValue - Like pushValueAndType, but where the type of the value is
 /// omitted (perhaps it was already encoded in an earlier operand).
 void DXILBitcodeWriter::pushValue(const Value *V, unsigned InstID,
void DXILBitcodeWriter::writeInstruction(const Instruction &I, unsigned InstID,
   unsigned AbbrevToUse = 0;
   VE.setInstructionID(&I);
   switch (I.getOpcode()) {
   default:
     if (Instruction::isCast(I.getOpcode())) {
       Code = bitc::FUNC_CODE_INST_CAST;
       if (!pushValueAndType(I.getOperand(0), InstID, Vals))
         AbbrevToUse = (unsigned)FUNCTION_INST_CAST_ABBREV;
-      Vals.push_back(VE.getTypeID(I.getType()));
+      Vals.push_back(getTypeID(I.getType(), &I));
       Vals.push_back(getEncodedCastOpcode(I.getOpcode()));
     } else {
       assert(isa<BinaryOperator>(I) && "Unknown instruction!");
       Code = bitc::FUNC_CODE_INST_BINOP;
       if (!pushValueAndType(I.getOperand(0), InstID, Vals))
         AbbrevToUse = (unsigned)FUNCTION_INST_BINOP_ABBREV;
       pushValue(I.getOperand(1), InstID, Vals);
       Vals.push_back(getEncodedBinaryOpcode(I.getOpcode()));
       uint64_t Flags = getOptimizationFlags(&I);
       if (Flags != 0) {
         if (AbbrevToUse == (unsigned)FUNCTION_INST_BINOP_ABBREV)
           AbbrevToUse = (unsigned)FUNCTION_INST_BINOP_FLAGS_ABBREV;
         Vals.push_back(Flags);
       }
     }
     break;
 
   case Instruction::GetElementPtr: {
     Code = bitc::FUNC_CODE_INST_GEP;
     AbbrevToUse = (unsigned)FUNCTION_INST_GEP_ABBREV;
     auto &GEPInst = cast<GetElementPtrInst>(I);
     Vals.push_back(GEPInst.isInBounds());
-    Vals.push_back(VE.getTypeID(GEPInst.getSourceElementType()));
+    Vals.push_back(getTypeID(GEPInst.getSourceElementType()));
     for (unsigned i = 0, e = I.getNumOperands(); i != e; ++i)
       pushValueAndType(I.getOperand(i), InstID, Vals);
     break;
   }
   case Instruction::ExtractValue: {
     Code = bitc::FUNC_CODE_INST_EXTRACTVAL;
     pushValueAndType(I.getOperand(0), InstID, Vals);
     const ExtractValueInst *EVI = cast<ExtractValueInst>(&I);
case Instruction::Br: {
     if (II.isConditional()) {
       Vals.push_back(VE.getValueID(II.getSuccessor(1)));
       pushValue(II.getCondition(), InstID, Vals);
     }
   } break;
   case Instruction::Switch: {
     Code = bitc::FUNC_CODE_INST_SWITCH;
     const SwitchInst &SI = cast<SwitchInst>(I);
-    Vals.push_back(VE.getTypeID(SI.getCondition()->getType()));
+    Vals.push_back(getTypeID(SI.getCondition()->getType()));
     pushValue(SI.getCondition(), InstID, Vals);
     Vals.push_back(VE.getValueID(SI.getDefaultDest()));
     for (auto Case : SI.cases()) {
       Vals.push_back(VE.getValueID(Case.getCaseValue()));
       Vals.push_back(VE.getValueID(Case.getCaseSuccessor()));
     }
   } break;
   case Instruction::IndirectBr:
     Code = bitc::FUNC_CODE_INST_INDIRECTBR;
-    Vals.push_back(VE.getTypeID(I.getOperand(0)->getType()));
+    Vals.push_back(getTypeID(I.getOperand(0)->getType()));
     // Encode the address operand as relative, but not the basic blocks.
     pushValue(I.getOperand(0), InstID, Vals);
     for (unsigned i = 1, e = I.getNumOperands(); i != e; ++i)
       Vals.push_back(VE.getValueID(I.getOperand(i)));
     break;
 
   case Instruction::Invoke: {
     const InvokeInst *II = cast<InvokeInst>(&I);
     const Value *Callee = II->getCalledOperand();
     FunctionType *FTy = II->getFunctionType();
     Code = bitc::FUNC_CODE_INST_INVOKE;
 
     Vals.push_back(VE.getAttributeListID(II->getAttributes()));
     Vals.push_back(II->getCallingConv() | 1 << 13);
     Vals.push_back(VE.getValueID(II->getNormalDest()));
     Vals.push_back(VE.getValueID(II->getUnwindDest()));
-    Vals.push_back(VE.getTypeID(FTy));
+    Vals.push_back(getTypeID(FTy));
     pushValueAndType(Callee, InstID, Vals);
 
     // Emit value #'s for the fixed parameters.
     for (unsigned i = 0, e = FTy->getNumParams(); i != e; ++i)
       pushValue(I.getOperand(i), InstID, Vals); // fixed param.
 
     // Emit type/value pairs for varargs params.
     if (FTy->isVarArg()) {
void DXILBitcodeWriter::writeInstruction(const Instruction &I, unsigned InstID,
 
   case Instruction::PHI: {
     const PHINode &PN = cast<PHINode>(I);
     Code = bitc::FUNC_CODE_INST_PHI;
     // With the newer instruction encoding, forward references could give
     // negative valued IDs.  This is most common for PHIs, so we use
     // signed VBRs.
     SmallVector<uint64_t, 128> Vals64;
-    Vals64.push_back(VE.getTypeID(PN.getType()));
+    Vals64.push_back(getTypeID(PN.getType()));
     for (unsigned i = 0, e = PN.getNumIncomingValues(); i != e; ++i) {
       pushValueSigned(PN.getIncomingValue(i), InstID, Vals64);
       Vals64.push_back(VE.getValueID(PN.getIncomingBlock(i)));
     }
     // Emit a Vals64 vector and exit.
     Stream.EmitRecord(Code, Vals64, AbbrevToUse);
     Vals64.clear();
     return;
   }
 
   case Instruction::LandingPad: {
     const LandingPadInst &LP = cast<LandingPadInst>(I);
     Code = bitc::FUNC_CODE_INST_LANDINGPAD;
-    Vals.push_back(VE.getTypeID(LP.getType()));
+    Vals.push_back(getTypeID(LP.getType()));
     Vals.push_back(LP.isCleanup());
     Vals.push_back(LP.getNumClauses());
     for (unsigned I = 0, E = LP.getNumClauses(); I != E; ++I) {
       if (LP.isCatch(I))
         Vals.push_back(LandingPadInst::Catch);
       else
         Vals.push_back(LandingPadInst::Filter);
       pushValueAndType(LP.getClause(I), InstID, Vals);
     }
     break;
   }
 
   case Instruction::Alloca: {
     Code = bitc::FUNC_CODE_INST_ALLOCA;
     const AllocaInst &AI = cast<AllocaInst>(I);
-    Vals.push_back(VE.getTypeID(AI.getAllocatedType()));
-    Vals.push_back(VE.getTypeID(I.getOperand(0)->getType()));
+    Vals.push_back(getTypeID(AI.getAllocatedType()));
+    Vals.push_back(getTypeID(I.getOperand(0)->getType()));
     Vals.push_back(VE.getValueID(I.getOperand(0))); // size.
     using APV = AllocaPackedValues;
     unsigned Record = 0;
     unsigned EncodedAlign = getEncodedAlign(AI.getAlign());
     Bitfield::set<APV::AlignLower>(
         Record, EncodedAlign & ((1 << APV::AlignLower::Bits) - 1));
     Bitfield::set<APV::AlignUpper>(Record,
                                    EncodedAlign >> APV::AlignLower::Bits);
     Bitfield::set<APV::UsedWithInAlloca>(Record, AI.isUsedWithInAlloca());
     Vals.push_back(Record);
     break;
   }
 
   case Instruction::Load:
     if (cast<LoadInst>(I).isAtomic()) {
       Code = bitc::FUNC_CODE_INST_LOADATOMIC;
       pushValueAndType(I.getOperand(0), InstID, Vals);
     } else {
       Code = bitc::FUNC_CODE_INST_LOAD;
       if (!pushValueAndType(I.getOperand(0), InstID, Vals)) // ptr
         AbbrevToUse = (unsigned)FUNCTION_INST_LOAD_ABBREV;
     }
-    Vals.push_back(VE.getTypeID(I.getType()));
+    Vals.push_back(getTypeID(I.getType()));
     Vals.push_back(Log2_32(cast<LoadInst>(I).getAlignment()) + 1);
     Vals.push_back(cast<LoadInst>(I).isVolatile());
     if (cast<LoadInst>(I).isAtomic()) {
       Vals.push_back(getEncodedOrdering(cast<LoadInst>(I).getOrdering()));
       Vals.push_back(getEncodedSyncScopeID(cast<LoadInst>(I).getSyncScopeID()));
     }
     break;
   case Instruction::Store:
case Instruction::Call: {
     const CallInst &CI = cast<CallInst>(I);
     FunctionType *FTy = CI.getFunctionType();
 
     Code = bitc::FUNC_CODE_INST_CALL;
 
     Vals.push_back(VE.getAttributeListID(CI.getAttributes()));
     Vals.push_back((CI.getCallingConv() << 1) | unsigned(CI.isTailCall()) |
                    unsigned(CI.isMustTailCall()) << 14 | 1 << 15);
-    Vals.push_back(VE.getTypeID(FTy));
+    Vals.push_back(getTypeID(FTy, CI.getCalledFunction()));
     pushValueAndType(CI.getCalledOperand(), InstID, Vals); // Callee
 
     // Emit value #'s for the fixed parameters.
     for (unsigned i = 0, e = FTy->getNumParams(); i != e; ++i) {
       // Check for labels (can happen with asm labels).
       if (FTy->getParamType(i)->isLabelTy())
         Vals.push_back(VE.getValueID(CI.getArgOperand(i)));
       else
         pushValue(CI.getArgOperand(i), InstID, Vals); // fixed param.
     }
 
     // Emit type/value pairs for varargs params.
     if (FTy->isVarArg()) {
       for (unsigned i = FTy->getNumParams(), e = CI.arg_size(); i != e; ++i)
         pushValueAndType(CI.getArgOperand(i), InstID, Vals); // varargs
     }
     break;
   }
   case Instruction::VAArg:
     Code = bitc::FUNC_CODE_INST_VAARG;
-    Vals.push_back(VE.getTypeID(I.getOperand(0)->getType())); // valistty
-    pushValue(I.getOperand(0), InstID, Vals);                 // valist.
-    Vals.push_back(VE.getTypeID(I.getType()));                // restype.
+    Vals.push_back(getTypeID(I.getOperand(0)->getType())); // valistty
+    pushValue(I.getOperand(0), InstID, Vals);              // valist.
+    Vals.push_back(getTypeID(I.getType()));                // restype.
     break;
   }
 
   Stream.EmitRecord(Code, Vals, AbbrevToUse);
   Vals.clear();
 }
 
 // Emit names for globals/functions etc.
else
     Code = bitc::USELIST_CODE_DEFAULT;
 
   SmallVector<uint64_t, 64> Record(Order.Shuffle.begin(), Order.Shuffle.end());
   Record.push_back(VE.getValueID(Order.V));
   Stream.EmitRecord(Code, Record);
 }
 
 void DXILBitcodeWriter::writeUseListBlock(const Function *F) {
-  assert(VE.shouldPreserveUseListOrder() &&
-         "Expected to be preserving use-list order");
-
   auto hasMore = [&]() {
     return !VE.UseListOrders.empty() && VE.UseListOrders.back().F == F;
   };
   if (!hasMore())
     // Nothing to do.
     return;
 
   Stream.EnterSubblock(bitc::USELIST_BLOCK_ID, 3);
for (Function::const_iterator BB = F.begin(), E = F.end(); BB != E; ++BB)
     }
 
   // Emit names for all the instructions etc.
   if (auto *Symtab = F.getValueSymbolTable())
     writeFunctionLevelValueSymbolTable(*Symtab);
 
   if (NeedsMetadataAttachment)
     writeFunctionMetadataAttachment(F);
-  if (VE.shouldPreserveUseListOrder())
-    writeUseListBlock(&F);
+  
   VE.purgeFunction();
   Stream.ExitBlock();
 }
 
 // Emit blockinfo, which defines the standard abbreviations etc.
 void DXILBitcodeWriter::writeBlockInfo() {
   // We only want to emit block info records for blocks that have multiple
   // instances: CONSTANTS_BLOCK, FUNCTION_BLOCK and VALUE_SYMTAB_BLOCK.

 
 void DXILBitcodeWriter::writeModuleVersion() {
   // VERSION: [version#]
   Stream.EmitRecord(bitc::MODULE_CODE_VERSION, ArrayRef<unsigned>{1});
 }
 
 /// WriteModule - Emit the specified module to the bitstream.
 void DXILBitcodeWriter::write() {
-  // The identification block is new since llvm-3.7, but the old bitcode reader
-  // will skip it.
-  // writeIdentificationBlock(Stream);
+  // The identification block is new since llvm-3.7, but the old bitcode
+  // reader will skip it. writeIdentificationBlock(Stream);
 
   Stream.EnterSubblock(bitc::MODULE_BLOCK_ID, 3);
 
   // It is redundant to fully-specify this here, but nice to make it explicit
   // so that it is clear the DXIL module version is different.
   DXILBitcodeWriter::writeModuleVersion();
 
   // Emit blockinfo, which defines the standard abbreviations etc.
   writeBlockInfo();
 
   // Emit information about attribute groups.
   writeAttributeGroupTable();
 
   // Emit information about parameter attributes.
   writeAttributeTable();
 
   // Emit information describing all of the types in the module.
   writeTypeTable();
 
   writeComdats();
 
-  // Emit top-level description of module, including target triple, inline asm,
-  // descriptors for global variables, and function prototype info.
+  // Emit top-level description of module, including target triple, inline
+  // asm, descriptors for global variables, and function prototype info.
   writeModuleInfo();
 
   // Emit constants.
   writeModuleConstants();
 
   // Emit metadata.
   writeModuleMetadataKinds();
 
   // Emit metadata.
   writeModuleMetadata();
 
   // Emit names for globals/functions etc.
   // DXIL uses the same format for module-level value symbol table as for the
   // function level table.
   writeFunctionLevelValueSymbolTable(M.getValueSymbolTable());
 
   // Emit module-level use-lists.
-  if (VE.shouldPreserveUseListOrder())
-    writeUseListBlock(nullptr);
 
   // Emit function bodies.
   for (const Function &F : M)
     if (!F.isDeclaration())
       writeFunction(F);
 
   Stream.ExitBlock();
 }

llvm/lib/Target/DirectX/DXILWriter/DXILValueEnumerator.h

struct MDRange {
     /// Number of strings in the prefix of the metadata range.
     unsigned NumStrings = 0;
 
     MDRange() = default;
     explicit MDRange(unsigned First) : First(First) {}
   };
   SmallDenseMap<unsigned, MDRange, 1> FunctionMDInfo;
 
-  bool ShouldPreserveUseListOrder;
-
   using AttributeGroupMapType = DenseMap<IndexAndAttrSet, unsigned>;
   AttributeGroupMapType AttributeGroupMap;
   std::vector<IndexAndAttrSet> AttributeGroups;
 
   using AttributeListMapType = DenseMap<AttributeList, unsigned>;
   AttributeListMapType AttributeListMap;
   std::vector<AttributeList> AttributeLists;
 
private:
   /// before incorporation.
   unsigned NumModuleMDs = 0;
   unsigned NumMDStrings = 0;
 
   unsigned FirstFuncConstantID;
   unsigned FirstInstID;
 
 public:
-  ValueEnumerator(const Module &M, bool ShouldPreserveUseListOrder);
+  ValueEnumerator(const Module &M, Type *PrefixType);
   ValueEnumerator(const ValueEnumerator &) = delete;
   ValueEnumerator &operator=(const ValueEnumerator &) = delete;
 
   void dump() const;
   void print(raw_ostream &OS, const ValueMapType &Map, const char *Name) const;
   void print(raw_ostream &OS, const MetadataMapType &Map,
              const char *Name) const;
 
   unsigned getValueID(const Value *V) const;
 
   unsigned getMetadataID(const Metadata *MD) const {
     auto ID = getMetadataOrNullID(MD);
     assert(ID != 0 && "Metadata not in slotcalculator!");
     return ID - 1;
   }
 
   unsigned getMetadataOrNullID(const Metadata *MD) const {
     return MetadataMap.lookup(MD).ID;
   }
 
   unsigned numMDs() const { return MDs.size(); }
 
-  bool shouldPreserveUseListOrder() const { return ShouldPreserveUseListOrder; }
-
   unsigned getTypeID(Type *T) const {
     TypeMapType::const_iterator I = TypeMap.find(T);
     assert(I != TypeMap.end() && "Type not in ValueEnumerator!");
     return I->second - 1;
   }
 
   unsigned getInstructionID(const Instruction *I) const;
   void setInstructionID(const Instruction *I);
public:
 
   /// incorporateFunction/purgeFunction - If you'd like to deal with a function,
   /// use these two methods to get its data into the ValueEnumerator!
   void incorporateFunction(const Function &F);
 
   void purgeFunction();
   uint64_t computeBitsRequiredForTypeIndicies() const;
 
+  void EnumerateType(Type *T);
+
 private:
-  void OptimizeConstants(unsigned CstStart, unsigned CstEnd);
 
   /// Reorder the reachable metadata.
   ///
   /// This is not just an optimization, but is mandatory for emitting MDString
   /// correctly.
   void organizeMetadata();
 
   /// Drop the function tag from the transitive operands of the given node.
private:
   void EnumerateFunctionLocalMetadata(const Function &F,
                                       const LocalAsMetadata *Local);
   void EnumerateFunctionLocalMetadata(unsigned F, const LocalAsMetadata *Local);
   void EnumerateFunctionLocalListMetadata(const Function &F,
                                           const DIArgList *ArgList);
   void EnumerateFunctionLocalListMetadata(unsigned F, const DIArgList *Arglist);
   void EnumerateNamedMDNode(const NamedMDNode *NMD);
   void EnumerateValue(const Value *V);
-  void EnumerateType(Type *T);
   void EnumerateOperandType(const Value *V);
   void EnumerateAttributes(AttributeList PAL);
 
   void EnumerateValueSymbolTable(const ValueSymbolTable &ST);
   void EnumerateNamedMetadata(const Module &M);
 };
 
 } // end namespace dxil
 } // end namespace llvm
 
 #endif // LLVM_DXILWRITER_VALUEENUMERATOR_H

llvm/lib/Target/DirectX/DXILWriter/DXILValueEnumerator.cpp

static UseListOrderStack predictUseListOrder(const Module &M) {
   for (const Function &F : M) {
     for (const Use &U : F.operands())
       predictValueUseListOrder(U.get(), nullptr, OM, Stack);
   }
 
   return Stack;
 }
 
-static bool isIntOrIntVectorValue(const std::pair<const Value *, unsigned> &V) {
-  return V.first->getType()->isIntOrIntVectorTy();
-}
+ValueEnumerator::ValueEnumerator(const Module &M, Type *PrefixType) {
+  EnumerateType(PrefixType);
   
-ValueEnumerator::ValueEnumerator(const Module &M,
-                                 bool ShouldPreserveUseListOrder)
-    : ShouldPreserveUseListOrder(ShouldPreserveUseListOrder) {
-  if (ShouldPreserveUseListOrder)
-    UseListOrders = predictUseListOrder(M);
 
   // Enumerate the global variables.
   for (const GlobalVariable &GV : M.globals()) {
     EnumerateValue(&GV);
     EnumerateType(GV.getValueType());
   }
 
   // Enumerate the functions.
ValueEnumerator::ValueEnumerator(const Module &M, Type *PrefixType) {
   }
 
   // Enumerate the ifuncs.
   for (const GlobalIFunc &GIF : M.ifuncs()) {
     EnumerateValue(&GIF);
     EnumerateType(GIF.getValueType());
   }
 
-  // Remember what is the cutoff between globalvalue's and other constants.
-  unsigned FirstConstant = Values.size();
-
   // Enumerate the global variable initializers and attributes.
   for (const GlobalVariable &GV : M.globals()) {
     if (GV.hasInitializer())
       EnumerateValue(GV.getInitializer());
     if (GV.hasAttributes())
       EnumerateAttributes(GV.getAttributesAsList(AttributeList::FunctionIndex));
   }
 
for (const BasicBlock &BB : F)
         // Don't enumerate the location directly -- it has a special record
         // type -- but enumerate its operands.
         if (DILocation *L = I.getDebugLoc())
           for (const Metadata *Op : L->operands())
             EnumerateMetadata(&F, Op);
       }
   }
 
-  // Optimize constant ordering.
-  OptimizeConstants(FirstConstant, Values.size());
-
   // Organize metadata ordering.
   organizeMetadata();
 }
 
 unsigned ValueEnumerator::getInstructionID(const Instruction *Inst) const {
   InstructionMapType::const_iterator I = InstructionMap.find(Inst);
   assert(I != InstructionMap.end() && "Instruction is not mapped!");
   return I->second;
for (const auto &I : Map) {
     const Metadata *MD = I.first;
     OS << "Metadata: slot = " << I.second.ID << "\n";
     OS << "Metadata: function = " << I.second.F << "\n";
     MD->print(OS);
     OS << "\n";
   }
 }
 
-/// OptimizeConstants - Reorder constant pool for denser encoding.
-void ValueEnumerator::OptimizeConstants(unsigned CstStart, unsigned CstEnd) {
-  if (CstStart == CstEnd || CstStart + 1 == CstEnd)
-    return;
-
-  if (ShouldPreserveUseListOrder)
-    // Optimizing constants makes the use-list order difficult to predict.
-    // Disable it for now when trying to preserve the order.
-    return;
-
-  std::stable_sort(Values.begin() + CstStart, Values.begin() + CstEnd,
-                   [this](const std::pair<const Value *, unsigned> &LHS,
-                          const std::pair<const Value *, unsigned> &RHS) {
-                     // Sort by plane.
-                     if (LHS.first->getType() != RHS.first->getType())
-                       return getTypeID(LHS.first->getType()) <
-                              getTypeID(RHS.first->getType());
-                     // Then by frequency.
-                     return LHS.second > RHS.second;
-                   });
-
-  // Ensure that integer and vector of integer constants are at the start of the
-  // constant pool.  This is important so that GEP structure indices come before
-  // gep constant exprs.
-  std::stable_partition(Values.begin() + CstStart, Values.begin() + CstEnd,
-                        isIntOrIntVectorValue);
-
-  // Rebuild the modified portion of ValueMap.
-  for (; CstStart != CstEnd; ++CstStart)
-    ValueMap[Values[CstStart].first] = CstStart + 1;
-}
-
 /// EnumerateValueSymbolTable - Insert all of the values in the specified symbol
 /// table into the values table.
 void ValueEnumerator::EnumerateValueSymbolTable(const ValueSymbolTable &VST) {
   for (ValueSymbolTable::const_iterator VI = VST.begin(), VE = VST.end();
        VI != VE; ++VI)
     EnumerateValue(VI->getValue());
 }
 
for (const Instruction &I : BB) {
       }
       if (auto *SVI = dyn_cast<ShuffleVectorInst>(&I))
         EnumerateValue(SVI->getShuffleMaskForBitcode());
     }
     BasicBlocks.push_back(&BB);
     ValueMap[&BB] = BasicBlocks.size();
   }
 
-  // Optimize the constant layout.
-  OptimizeConstants(FirstFuncConstantID, Values.size());
-
   // Add the function's parameter attributes so they are available for use in
   // the function's instruction.
   EnumerateAttributes(F.getAttributes());
 
   FirstInstID = Values.size();
 
   SmallVector<LocalAsMetadata *, 8> FnLocalMDVector;
   SmallVector<DIArgList *, 8> ArgListMDVector;

llvm/test/tools/dxil-dis/opaque-gep.ll

+; RUN: llc --filetype=obj %s -o - | dxil-dis -o - | FileCheck %s
+target triple = "dxil-unknown-unknown"
+
+define i32 @fn(ptr %0)  {
+  %2 = getelementptr i32, ptr %0, i32 4
+  %3 = load i32, ptr %2
+  ret i32 %3
+}
+
+; CHECK:        define i32 @fn(i32*) 
+; CHECK-NEXT:   %2 = getelementptr i32, i32* %0, i32 4
+; CHECK-NEXT:   %3 = load i32, i32* %2, align 4

llvm/test/tools/dxil-dis/opaque-pointers.ll

+; RUN: llc --filetype=obj %s -o - | dxil-dis -o - | FileCheck %s
+target triple = "dxil-unknown-unknown"
+
+define i64 @test(ptr %p) {
+  store i32 0, ptr %p
+  %v = load i64, ptr %p
+  ret i64 %v
+}
+
+; CHECK: define i64 @test(i8* %p) {
+; CHECK-NEXT: %1 = bitcast i8* %p to i64*
+; CHECK-NEXT: %2 = bitcast i8* %p to i32*
+; CHECK-NEXT: store i32 0, i32* %2, align 4
+; CHECK-NEXT: %3 = load i64, i64* %1, align 8
+
+define i64 @test2(ptr %p) {
+  store i64 0, ptr %p
+  %v = load i64, ptr %p
+  ret i64 %v
+}
+
+; CHECK: define i64 @test2(i64* %p) {
+; CHECK-NEXT: store i64 0, i64* %p, align 8
+; CHECK-NEXT: %v = load i64, i64* %p, align 8