Use a SmallVector of pairs instead of DenseMap to collect Function

pointers and corresponding shader flag masks. This follows the
recommendations in LLVM Programmer's Manual as the current usage
pattern has distinct phases of insertion of computed shader flags
followed by querying. Upon insertion, the Smallvector is sorted and
binary search is used for querying. Necessary comparison function of
pairs is also implemented.

Added a simple DiagnosticInfoShaderFlags for emitting diagnostics.

Added tests to verify shader flags masks collected per-function.

Author: bharadwajy

llvm/lib/Target/DirectX/DXContainerGlobals.cpp

@@ -84,7 +84,7 @@ GlobalVariable *DXContainerGlobals::getFeatureFlags(Module &M) {
   // of the shader flags of all functions in the module. Need to verify
   // and modify the computation of feature flags to be used.
   uint64_t ConsolidatedFeatureFlags = 0;
-  for (const auto &FuncFlags : MSFI.FuncShaderFlagsMap) {
+  for (const auto &FuncFlags : MSFI.FuncShaderFlagsVec) {
     ConsolidatedFeatureFlags |= FuncFlags.second.getFeatureFlags();
   }
 

llvm/lib/Target/DirectX/DXILShaderFlags.cpp

@@ -13,43 +13,114 @@
 
 #include "DXILShaderFlags.h"
 #include "DirectX.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/IR/DiagnosticInfo.h"
+#include "llvm/IR/DiagnosticPrinter.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
 using namespace llvm::dxil;
 
-static void updateFlags(DXILModuleShaderFlagsInfo &MSFI, const Instruction &I) {
-  ComputedShaderFlags &FSF = MSFI.FuncShaderFlagsMap[I.getFunction()];
+namespace {
+/// A simple Wrapper DiagnosticInfo that generates Module-level diagnostic
+/// for ShaderFlagsAnalysis pass
+class DiagnosticInfoShaderFlags : public DiagnosticInfo {
+private:
+  const Twine &Msg;
+  const Module &Mod;
+
+public:
+  /// \p M is the module for which the diagnostic is being emitted. \p Msg is
+  /// the message to show. Note that this class does not copy this message, so
+  /// this reference must be valid for the whole life time of the diagnostic.
+  DiagnosticInfoShaderFlags(const Module &M, const Twine &Msg,
+                            DiagnosticSeverity Severity = DS_Error)
+      : DiagnosticInfo(DK_Unsupported, Severity), Msg(Msg), Mod(M) {}
+
+  void print(DiagnosticPrinter &DP) const override {
+    DP << Mod.getName() << ": " << Msg << '\n';
+  }
+};
+} // namespace
+
+static void updateFlags(ComputedShaderFlags &CSF, const Instruction &I) {
   Type *Ty = I.getType();
-  if (Ty->isDoubleTy()) {
-    FSF.Doubles = true;
+  bool DoubleTyInUse = Ty->isDoubleTy();
+  for (Value *Op : I.operands()) {
+    DoubleTyInUse |= Op->getType()->isDoubleTy();
+  }
+
+  if (DoubleTyInUse) {
+    CSF.Doubles = true;
     switch (I.getOpcode()) {
     case Instruction::FDiv:
     case Instruction::UIToFP:
     case Instruction::SIToFP:
     case Instruction::FPToUI:
     case Instruction::FPToSI:
-      FSF.DX11_1_DoubleExtensions = true;
+      // TODO: To be set if I is a call to DXIL intrinsic DXIL::Opcode::Fma
+      CSF.DX11_1_DoubleExtensions = true;
       break;
     }
   }
 }
 
+static bool compareFuncSFPairs(const FuncShaderFlagsMask &First,
+                               const FuncShaderFlagsMask &Second) {
+  // Construct string representation of the functions in each pair
+  // as "retTypefunctionNamearg1Typearg2Ty..." where the function signature is
+  // retType functionName(arg1Type, arg2Ty,...).  Spaces, braces and commas are
+  //  omitted in the string representation of the signature. This allows
+  // determining a consistent lexicographical order of all functions by their
+  // signatures.
+  std::string FirstFunSig;
+  std::string SecondFunSig;
+  raw_string_ostream FRSO(FirstFunSig);
+  raw_string_ostream SRSO(SecondFunSig);
+
+  // Return type
+  First.first->getReturnType()->print(FRSO);
+  Second.first->getReturnType()->print(SRSO);
+  // Function name
+  FRSO << First.first->getName();
+  SRSO << Second.first->getName();
+  // Argument types
+  for (const Argument &Arg : First.first->args()) {
+    Arg.getType()->print(FRSO);
+  }
+  for (const Argument &Arg : Second.first->args()) {
+    Arg.getType()->print(SRSO);
+  }
+  FRSO.flush();
+  SRSO.flush();
+
+  return FRSO.str().compare(SRSO.str()) < 0;
+}
+
 static DXILModuleShaderFlagsInfo computeFlags(Module &M) {
   DXILModuleShaderFlagsInfo MSFI;
-  for (const auto &F : M) {
+  for (auto &F : M) {
     if (F.isDeclaration())
       continue;
-    if (!MSFI.FuncShaderFlagsMap.contains(&F)) {
-      ComputedShaderFlags CSF{};
-      MSFI.FuncShaderFlagsMap[&F] = CSF;
+    // Each of the functions in a module are unique. Hence no prior shader flags
+    // mask of the function should be present.
+    if (MSFI.hasShaderFlagsMask(&F)) {
+      M.getContext().diagnose(DiagnosticInfoShaderFlags(
+          M, "Shader Flags mask for Function '" + Twine(F.getName()) +
+                 "' already exits"));
     }
+    ComputedShaderFlags CSF{};
     for (const auto &BB : F)
       for (const auto &I : BB)
-        updateFlags(MSFI, I);
+        updateFlags(CSF, I);
+    // Insert shader flag mask for function F
+    MSFI.FuncShaderFlagsVec.push_back({&F, CSF});
   }
+  // Sort MSFI.FuncShaderFlagsVec for later lookup that uses binary search
+  llvm::sort(MSFI.FuncShaderFlagsVec, compareFuncSFPairs);
   return MSFI;
 }
 
@@ -71,6 +142,38 @@ void ComputedShaderFlags::print(raw_ostream &OS) const {
   OS << ";\n";
 }
 
+void DXILModuleShaderFlagsInfo::print(raw_ostream &OS) const {
+  OS << "; Shader Flags mask for Module:\n";
+  ModuleFlags.print(OS);
+  for (auto SF : FuncShaderFlagsVec) {
+    OS << "; Shader Flags mask for Function: " << SF.first->getName() << "\n";
+    SF.second.print(OS);
+  }
+}
+
+const ComputedShaderFlags
+DXILModuleShaderFlagsInfo::getShaderFlagsMask(const Function *Func) const {
+  FuncShaderFlagsMask V{Func, {}};
+  auto Iter = llvm::lower_bound(FuncShaderFlagsVec, V, compareFuncSFPairs);
+  if (Iter == FuncShaderFlagsVec.end()) {
+    Func->getContext().diagnose(DiagnosticInfoShaderFlags(
+        *(Func->getParent()), "Shader Flags information of Function '" +
+                                  Twine(Func->getName()) + "' not found"));
+  }
+  if (Iter->first != Func) {
+    Func->getContext().diagnose(DiagnosticInfoShaderFlags(
+        *(Func->getParent()),
+        "Inconsistent Shader Flags information of Function '" +
+            Twine(Func->getName()) + "' retrieved"));
+  }
+  return Iter->second;
+}
+
+bool DXILModuleShaderFlagsInfo::hasShaderFlagsMask(const Function *Func) const {
+  FuncShaderFlagsMask V{Func, {}};
+  return llvm::binary_search(FuncShaderFlagsVec, V);
+}
+
 AnalysisKey ShaderFlagsAnalysis::Key;
 
 DXILModuleShaderFlagsInfo ShaderFlagsAnalysis::run(Module &M,
@@ -86,12 +189,7 @@ bool ShaderFlagsAnalysisWrapper::runOnModule(Module &M) {
 PreservedAnalyses ShaderFlagsAnalysisPrinter::run(Module &M,
                                                   ModuleAnalysisManager &AM) {
   DXILModuleShaderFlagsInfo Flags = AM.getResult<ShaderFlagsAnalysis>(M);
-  OS << "; Shader Flags mask for Module:\n";
-  Flags.ModuleFlags.print(OS);
-  for (auto SF : Flags.FuncShaderFlagsMap) {
-    OS << "; Shader Flags mash for Function: " << SF.first->getName() << "\n";
-    SF.second.print(OS);
-  }
+  Flags.print(OS);
   return PreservedAnalyses::all();
 }
 

llvm/lib/Target/DirectX/DXILShaderFlags.h

@@ -14,7 +14,6 @@
 #ifndef LLVM_TARGET_DIRECTX_DXILSHADERFLAGS_H
 #define LLVM_TARGET_DIRECTX_DXILSHADERFLAGS_H
 
-#include "llvm/ADT/DenseMap.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/Pass.h"
@@ -66,14 +65,18 @@ struct ComputedShaderFlags {
   LLVM_DUMP_METHOD void dump() const { print(); }
 };
 
-using FunctionShaderFlagsMap =
-    SmallDenseMap<Function const *, ComputedShaderFlags>;
+using FuncShaderFlagsMask = std::pair<Function const *, ComputedShaderFlags>;
+using FunctionShaderFlagsVec = SmallVector<FuncShaderFlagsMask>;
 struct DXILModuleShaderFlagsInfo {
   // Shader Flag mask representing module-level properties
   ComputedShaderFlags ModuleFlags;
-  // Map representing shader flag mask representing properties of each of the
-  // functions in the module
-  FunctionShaderFlagsMap FuncShaderFlagsMap;
+  // Vector of Function-Shader Flag mask pairs representing properties of each
+  // of the functions in the module
+  FunctionShaderFlagsVec FuncShaderFlagsVec;
+
+  const ComputedShaderFlags getShaderFlagsMask(const Function *Func) const;
+  bool hasShaderFlagsMask(const Function *Func) const;
+  void print(raw_ostream &OS = dbgs()) const;
 };
 
 class ShaderFlagsAnalysis : public AnalysisInfoMixin<ShaderFlagsAnalysis> {

llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp

@@ -318,7 +318,7 @@ static void translateMetadata(Module &M, const DXILResourceMap &DRM,
     // to be used as shader flags mask value associated with top-level library
     // entry metadata.
     uint64_t ConsolidatedMask = ShaderFlags.ModuleFlags;
-    for (const auto &FunFlags : ShaderFlags.FuncShaderFlagsMap) {
+    for (const auto &FunFlags : ShaderFlags.FuncShaderFlagsVec) {
       ConsolidatedMask |= FunFlags.second;
     }
     EntryFnMDNodes.emplace_back(
@@ -329,20 +329,15 @@ static void translateMetadata(Module &M, const DXILResourceMap &DRM,
   }
 
   for (const EntryProperties &EntryProp : MMDI.EntryPropertyVec) {
-    auto FSFIt = ShaderFlags.FuncShaderFlagsMap.find(EntryProp.Entry);
-    if (FSFIt == ShaderFlags.FuncShaderFlagsMap.end()) {
-      M.getContext().diagnose(DiagnosticInfoTranslateMD(
-          M, "Shader Flags of Function '" + Twine(EntryProp.Entry->getName()) +
-                 "' not found"));
-    }
+    ComputedShaderFlags ECSF = ShaderFlags.getShaderFlagsMask(EntryProp.Entry);
     // If ShaderProfile is Library, mask is already consolidated in the
     // top-level library node. Hence it is not emitted.
     uint64_t EntryShaderFlags = 0;
     if (MMDI.ShaderProfile != Triple::EnvironmentType::Library) {
       // TODO: Create a consolidated shader flag mask of all the entry
       // functions and its callees. The following is correct only if
-      // (*FSIt).first has no call instructions.
-      EntryShaderFlags = (*FSFIt).second | ShaderFlags.ModuleFlags;
+      // EntryProp.Entry has no call instructions.
+      EntryShaderFlags = ECSF | ShaderFlags.ModuleFlags;
     }
     if (MMDI.ShaderProfile != Triple::EnvironmentType::Library) {
       if (EntryProp.ShaderStage != MMDI.ShaderProfile) {

llvm/test/CodeGen/DirectX/ShaderFlags/double-extensions-obj-test.ll

@@ -0,0 +1,19 @@
+; RUN: llc %s --filetype=obj -o - | obj2yaml | FileCheck %s --check-prefix=DXC
+
+target triple = "dxil-pc-shadermodel6.7-library"
+define double @div(double %a, double %b) #0 {
+  %res = fdiv double %a, %b
+  ret double %res
+}
+
+attributes #0 = { convergent norecurse nounwind "hlsl.export"}
+
+; DXC: - Name:            SFI0
+; DXC-NEXT:     Size:            8
+; DXC-NEXT:     Flags:
+; DXC-NEXT:       Doubles:         true
+; DXC-NOT:   {{[A-Za-z]+: +true}}
+; DXC:            DX11_1_DoubleExtensions:         true
+; DXC-NOT:   {{[A-Za-z]+: +true}}
+; DXC:       NextUnusedBit:   false
+; DXC: ...

llvm/test/CodeGen/DirectX/ShaderFlags/double-extensions.ll

@@ -1,27 +1,74 @@
 ; RUN: opt -S --passes="print-dx-shader-flags" 2>&1 %s | FileCheck %s
-; RUN: llc %s --filetype=obj -o - | obj2yaml | FileCheck %s --check-prefix=DXC
 
 target triple = "dxil-pc-shadermodel6.7-library"
 
-; CHECK: ; Shader Flags Value: 0x00000044
-; CHECK: ; Note: shader requires additional functionality:
+; CHECK: ; Shader Flags mask for Module:
+; CHECK-NEXT: ; Shader Flags Value: 0x00000000
+; CHECK-NEXT: ;
+; CHECK-NEXT: ; Shader Flags mask for Function: test_fdiv_double
+; CHECK-NEXT: ; Shader Flags Value: 0x00000044
+; CHECK-NEXT: ;
+; CHECK-NEXT: ; Note: shader requires additional functionality:
 ; CHECK-NEXT: ;       Double-precision floating point
 ; CHECK-NEXT: ;       Double-precision extensions for 11.1
 ; CHECK-NEXT: ; Note: extra DXIL module flags:
-; CHECK-NEXT: {{^;$}}
-define double @div(double %a, double %b) #0 {
+; CHECK-NEXT: ;
+; CHECK-NEXT: ; Shader Flags mask for Function: test_sitofp_i64
+; CHECK-NEXT: ; Shader Flags Value: 0x00000044
+; CHECK-NEXT: ;
+; CHECK-NEXT: ; Note: shader requires additional functionality:
+; CHECK-NEXT: ;       Double-precision floating point
+; CHECK-NEXT: ;       Double-precision extensions for 11.1
+; CHECK-NEXT: ; Note: extra DXIL module flags:
+; CHECK-NEXT: ;
+; CHECK-NEXT: ; Shader Flags mask for Function: test_uitofp_i64
+; CHECK-NEXT: ; Shader Flags Value: 0x00000044
+; CHECK-NEXT: ;
+; CHECK-NEXT: ; Note: shader requires additional functionality:
+; CHECK-NEXT: ;       Double-precision floating point
+; CHECK-NEXT: ;       Double-precision extensions for 11.1
+; CHECK-NEXT: ; Note: extra DXIL module flags:
+; CHECK-NEXT: ;
+; CHECK-NEXT: ; Shader Flags mask for Function: test_fptoui_i32
+; CHECK-NEXT: ; Shader Flags Value: 0x00000044
+; CHECK-NEXT: ;
+; CHECK-NEXT: ; Note: shader requires additional functionality:
+; CHECK-NEXT: ;       Double-precision floating point
+; CHECK-NEXT: ;       Double-precision extensions for 11.1
+; CHECK-NEXT: ; Note: extra DXIL module flags:
+; CHECK-NEXT: ;
+; CHECK-NEXT: ; Shader Flags mask for Function: test_fptosi_i64
+; CHECK-NEXT: ; Shader Flags Value: 0x00000044
+; CHECK-NEXT: ;
+; CHECK-NEXT: ; Note: shader requires additional functionality:
+; CHECK-NEXT: ;       Double-precision floating point
+; CHECK-NEXT: ;       Double-precision extensions for 11.1
+; CHECK-NEXT: ; Note: extra DXIL module flags:
+; CHECK-NEXT: ;
+
+define double @test_fdiv_double(double %a, double %b) #0 {
   %res = fdiv double %a, %b
   ret double %res
 }
 
-attributes #0 = { convergent norecurse nounwind "hlsl.export"}
+define double @test_uitofp_i64(i64 %a) #0 {
+  %r = uitofp i64 %a to double
+  ret double %r
+}
+
+define double @test_sitofp_i64(i64 %a) #0 {
+  %r = sitofp i64 %a to double
+  ret double %r
+}
 
-; DXC: - Name:            SFI0
-; DXC-NEXT:     Size:            8
-; DXC-NEXT:     Flags:
-; DXC-NEXT:       Doubles:         true
-; DXC-NOT:   {{[A-Za-z]+: +true}}
-; DXC:            DX11_1_DoubleExtensions:         true
-; DXC-NOT:   {{[A-Za-z]+: +true}}
-; DXC:       NextUnusedBit:   false
-; DXC: ...
+define i32 @test_fptoui_i32(double %a) #0 {
+  %r = fptoui double %a to i32
+  ret i32 %r
+}
+
+define i64 @test_fptosi_i64(double %a) #0 {
+  %r = fptosi double %a to i64
+  ret i64 %r
+}
+
+attributes #0 = { convergent norecurse nounwind "hlsl.export"}

llvm/test/CodeGen/DirectX/ShaderFlags/doubles.ll

@@ -3,7 +3,10 @@
 
 target triple = "dxil-pc-shadermodel6.7-library"
 
-; CHECK: ; Shader Flags Value: 0x00000004
+; CHECK: ; Shader Flags mask for Module:
+; CHECK-NEXT: ; Shader Flags Value: 0x00000000
+; CHECK: ; Shader Flags mask for Function: add
+; CHECK-NEXT: ; Shader Flags Value: 0x00000004
 ; CHECK: ; Note: shader requires additional functionality:
 ; CHECK-NEXT: ;       Double-precision floating point
 ; CHECK-NEXT: ; Note: extra DXIL module flags:

llvm/test/CodeGen/DirectX/ShaderFlags/no_flags.ll

@@ -2,7 +2,11 @@
 
 target triple = "dxil-pc-shadermodel6.7-library"
 
-; CHECK: ; Shader Flags Value: 0x00000000
+; CHECK: ; Shader Flags mask for Module:
+; CHECK-NEXT: ; Shader Flags Value: 0x00000000
+;
+; CHECK: ; Shader Flags mask for Function: add
+; CHECK-NEXT: ; Shader Flags Value: 0x00000000
 define i32 @add(i32 %a, i32 %b) {
   %sum = add i32 %a, %b
   ret i32 %sum