Localize only global variables in private addrspace.

dmitryryintel · igcbot · commit 6bfd67bf98b5 · 2021-07-16T23:39:32.000+02:00
Prior to this patch global variable localization was considered an
optimization. GV was localized based on heuristics, which is wrong. GV
can be localized only when GV is thread local (or one can call it kernel
local) and it must be localized in this case since current toolchain
doesn't have more general thread local GV support. Non thread local
GVs cannot be localized since it would change their behavior.
Being thread local is defined via addrspace: GVs in private addrspace
are considered thread local since private memory is not available to
other threads, GVs in other addrspaces are ignored by the localization.
LLVM thread local attribute is currently ignored.
diff --git a/IGC/Options/include/igc/Options/VCInternalOptions.td b/IGC/Options/include/igc/Options/VCInternalOptions.td
@@ -48,14 +48,5 @@ def : PlainJoined<"runtime=">, Alias<binary_format>,
 def ftime_report : PlainFlag<"ftime-report">,
   HelpText<"Print timing summary of each stage of compilation">;
 
-def vc_globals_localization : PlainSeparate<"globals-localization">,
-  HelpText<"globals localization optimization configuration; values:\n"
-           "\t'all' - localize all globals,\n\t'no' - don't localize globals,\n"
-           "\t'vector' - localize all vector globals the rest is partially "
-           "localized,\n\t'partial' - compiler may localize some globals "
-           "according to its heuristics">;
-def : PlainJoined<"globals-localization=">, Alias<vc_globals_localization>,
-  HelpText<"Alias for -globals-localization <value>">;
-
 }
 // }} VC internal options
diff --git a/IGC/VectorCompiler/include/vc/Driver/Driver.h b/IGC/VectorCompiler/include/vc/Driver/Driver.h
@@ -86,7 +86,6 @@ struct CompileOptions {
   bool DumpAsm = false;
   bool DumpDebugInfo = false;
   bool TimePasses = false;
-  GlobalsLocalizationMode GlobalsLocalization = GlobalsLocalizationMode::Vector;
   std::string LLVMOptions;
   bool UseBindlessBuffers = false;
 
diff --git a/IGC/VectorCompiler/include/vc/Support/BackendConfig.h b/IGC/VectorCompiler/include/vc/Support/BackendConfig.h
@@ -46,55 +46,6 @@ namespace llvm {
 
 void initializeGenXBackendConfigPass(PassRegistry &PR);
 
-struct GlobalsLocalizationConfig {
-  using LimitT = std::size_t;
-  static constexpr auto NoLimit = std::numeric_limits<LimitT>::max();
-
-private:
-  // Whether every global variable must be localized.
-  bool IsForced = true;
-  // Whether every vector global variable must be localized.
-  bool IsVectorForced = true;
-  // How many GRF memory is allowed to be used for localization.
-  LimitT Limit = NoLimit;
-
-public:
-  GlobalsLocalizationConfig(bool IsForcedIn, bool IsVectorForcedIn,
-                            LimitT LimitIn)
-      : IsForced{IsForcedIn}, IsVectorForced{IsVectorForcedIn}, Limit{LimitIn} {
-    if (IsForced || IsVectorForced)
-      IGC_ASSERT_MESSAGE(
-          Limit == NoLimit,
-          "there can be no localization limit when localization is forced");
-    if (IsForced)
-      IGC_ASSERT_MESSAGE(IsVectorForced,
-                         "localizing every GV means localizing vectors too");
-  }
-
-  GlobalsLocalizationConfig() {}
-
-  // Every global variable must be localized.
-  static GlobalsLocalizationConfig CreateForcedLocalization() { return {}; }
-
-  // Every global variable must be localized.
-  static GlobalsLocalizationConfig CreateForcedVectorLocalization() {
-    return {/* IsForced */ false, /* IsVectorForced */ true, NoLimit};
-  }
-
-  // GlobalsLocalization is allowed to localize globals but it can use only
-  // GlobalsLocalizationLimit bytes of GRF.
-  static GlobalsLocalizationConfig
-  CreateLocalizationWithLimit(LimitT GlobalsLocalizationLimitIn = NoLimit) {
-    return {/* IsForced */ false, /* IsVectorForced */ false,
-            GlobalsLocalizationLimitIn};
-  }
-
-  bool isForced() const { return IsForced; }
-  bool isVectorForced() const { return IsVectorForced; }
-
-  LimitT getLimit() const { return Limit; }
-};
-
 // Plain structure to be filled by users who want to create backend
 // configuration. Some values are default-initialized from cl options.
 struct GenXBackendOptions {
@@ -120,9 +71,6 @@ struct GenXBackendOptions {
   bool EnableDebugInfoDumps;
   std::string DebugInfoDumpsNameOverride;
 
-  // Configuration for GlobalsLocalization pass
-  // (part of CMABI pass by historical reasons).
-  GlobalsLocalizationConfig GlobalsLocalization;
   bool ForceArrayPromotion = false;
 
   // Localize live ranges to reduce accumulator usage
@@ -235,15 +183,6 @@ class GenXBackendConfig : public ImmutablePass {
     return Options.DebugInfoDumpsNameOverride;
   }
 
-  bool isGlobalsLocalizationForced() const {
-    return Options.GlobalsLocalization.isForced();
-  }
-  bool isVectorGlobalsLocalizationForced() const {
-    return Options.GlobalsLocalization.isVectorForced();
-  }
-  GlobalsLocalizationConfig::LimitT getGlobalsLocalizationLimit() const {
-    return Options.GlobalsLocalization.getLimit();
-  }
   bool isArrayPromotionForced() const { return Options.ForceArrayPromotion; }
 
   bool localizeLiveRangesForAccUsage() const {
diff --git a/IGC/VectorCompiler/lib/Driver/Driver.cpp b/IGC/VectorCompiler/lib/Driver/Driver.cpp
@@ -198,26 +198,6 @@ createTargetMachine(const vc::CompileOptions &Opts, Triple &TheTriple) {
   return {std::move(TM)};
 }
 
-static GlobalsLocalizationConfig
-defineGlobalsLocalizationConfig(vc::GlobalsLocalizationMode GLMode,
-                                vc::BinaryKind Binary) {
-  // Globals must be forced for CMRT binary.
-  if (Binary == vc::BinaryKind::CM)
-    return GlobalsLocalizationConfig::CreateForcedLocalization();
-  switch (GLMode) {
-  case vc::GlobalsLocalizationMode::All:
-    return GlobalsLocalizationConfig::CreateForcedLocalization();
-  case vc::GlobalsLocalizationMode::No:
-    return GlobalsLocalizationConfig::CreateLocalizationWithLimit(0);
-  case vc::GlobalsLocalizationMode::Vector:
-    return GlobalsLocalizationConfig::CreateForcedVectorLocalization();
-  default:
-    IGC_ASSERT_MESSAGE(GLMode == vc::GlobalsLocalizationMode::Partial,
-                       "unexpected globals localization mode");
-    return GlobalsLocalizationConfig::CreateLocalizationWithLimit();
-  }
-}
-
 // Create backend options for immutable config pass. Override default
 // values with provided ones.
 static GenXBackendOptions createBackendOptions(const vc::CompileOptions &Opts) {
@@ -233,8 +213,6 @@ static GenXBackendOptions createBackendOptions(const vc::CompileOptions &Opts) {
   BackendOpts.EnableDebugInfoDumps = Opts.DumpDebugInfo;
   BackendOpts.Dumper = Opts.Dumper.get();
   BackendOpts.ShaderOverrider = Opts.ShaderOverrider.get();
-  BackendOpts.GlobalsLocalization =
-      defineGlobalsLocalizationConfig(Opts.GlobalsLocalization, Opts.Binary);
   BackendOpts.ForceArrayPromotion = (Opts.Binary == vc::BinaryKind::CM);
   if (Opts.ForceLiveRangesLocalizationForAccUsage)
     BackendOpts.LocalizeLRsForAccUsage = true;
@@ -619,21 +597,6 @@ static Error fillInternalOptions(const opt::ArgList &InternalOptions,
     Opts.Binary = MaybeBinary.getValue();
   }
 
-  if (opt::Arg *A = InternalOptions.getLastArg(OPT_vc_globals_localization)) {
-    StringRef Val = A->getValue();
-    auto MaybeGLM = StringSwitch<Optional<vc::GlobalsLocalizationMode>>(Val)
-                        .Case("all", vc::GlobalsLocalizationMode::All)
-                        .Case("no", vc::GlobalsLocalizationMode::No)
-                        .Case("vector", vc::GlobalsLocalizationMode::Vector)
-                        .Case("partial", vc::GlobalsLocalizationMode::Partial)
-                        .Default(None);
-    // FIXME: -globals-localization=no is ignored when cm binary is used, throw
-    //        a warning here.
-    if (!MaybeGLM)
-      return makeOptionError(*A, InternalOptions, /*IsInternal=*/true);
-    Opts.GlobalsLocalization = MaybeGLM.getValue();
-  }
-
   Opts.FeaturesString =
       llvm::join(InternalOptions.getAllArgValues(OPT_target_features), ",");
 
diff --git a/IGC/VectorCompiler/lib/GenXOpts/CMTrans/CMABI.cpp b/IGC/VectorCompiler/lib/GenXOpts/CMTrans/CMABI.cpp
@@ -239,8 +239,6 @@ int DiagnosticInfoOverlappingArgs::KindID = 0;
 class CMABIAnalysis : public ModulePass {
   // This map captures all global variables to be localized.
   std::vector<LocalizationInfo *> LocalizationInfoObjs;
-  GlobalsLocalizationConfig::LimitT GlobalsLocalizationLimit = 0;
-  bool LocalizeVectorGlobals = false;
 
 public:
   static char ID;
@@ -355,118 +353,14 @@ INITIALIZE_PASS_DEPENDENCY(GenXBackendConfig)
 INITIALIZE_PASS_END(CMABIAnalysis, "cmabi-analysis",
                     "Fix ABI issues for the genx backend", false, true)
 
-static std::size_t
-defineGlobalsLocalizationLimit(const GenXBackendConfig &Config) {
-  if (Config.isGlobalsLocalizationForced())
-    return GlobalsLocalizationConfig::NoLimit;
-
-  // Half of a size of standard GenX register file in bytes.
-  // 128 * 32 / 2
-  constexpr std::size_t HalfGRF = 2048;
-  std::size_t Limit = Config.getGlobalsLocalizationLimit();
-  return std::min(Limit, HalfGRF);
-}
-
 bool CMABIAnalysis::runOnModule(Module &M) {
   auto &&BCfg = getAnalysis<GenXBackendConfig>();
-  GlobalsLocalizationLimit = defineGlobalsLocalizationLimit(BCfg);
-  LocalizeVectorGlobals = BCfg.isVectorGlobalsLocalizationForced();
   FCtrl = BCfg.getFCtrl();
 
   runOnCallGraph(getAnalysis<CallGraphWrapperPass>().getCallGraph());
   return false;
 }
 
-// Currently weight of the global defines by its size
-static int calcGVWeight(const GlobalVariable &GV, const DataLayout &DL) {
-  if (!GV.getValueType()->isVectorTy())
-    return DL.getTypeAllocSize(GV.getValueType());
-  // Alignment rules are too restrictive for vectors and cannot be reduced
-  // (even though LangRef says that they can).
-  // GRF width (32) alignment was taken as it is considered that localized
-  // global vectors should be eventually promoted to GRF.
-  return alignTo(DL.getTypeStoreSize(GV.getValueType()), 32);
-}
-
-// selectGlobalsToLocalize - chooses which globals to localize.
-// Returns std::vector of std::reference_wrapper to such globals.
-//
-// Algorithm: exclude globals that definitely should not be localized, include
-// those that definitely should. If the total weight of the already chosen
-// globals doesn't exceed \p Bound, sort the remaining globals by weight,
-// choose first lightest ones, so the total weight is under \p Bound.
-//
-// \p Globals - range of globals to choose from
-// \p Bound - bound not to overcome
-// \p ExcludePred - functor : GVRef -> bool, true if global should not be
-//                  localized
-// \p InlcudePred - functor : GVRef -> bool, true if the provided global must
-//                  be localized
-// \p WeightCalculator - functor : GVRef -> decltype(Bound), returns
-// weight of global
-template <typename ForwardRange, typename ExcludePredT, typename IncludePredT,
-          typename T, typename WeightCalculatorT>
-auto selectGlobalsToLocalize(ForwardRange Globals, T Bound,
-                             ExcludePredT ExcludePred, IncludePredT IncludePred,
-                             WeightCalculatorT WeightCalculator) {
-  IGC_ASSERT_MESSAGE(Bound >= 0, "bound must be nonnegative");
-  using GVRef = vc::ranges::range_reference_t<ForwardRange>;
-  using GVT = std::remove_reference_t<GVRef>;
-  using GVRefWrapper = std::reference_wrapper<GVT>;
-
-  IGC_ASSERT_MESSAGE(std::none_of(Globals.begin(), Globals.end(),
-                                  [ExcludePred, IncludePred](GVRef GV) {
-                                    return ExcludePred(GV) && IncludePred(GV);
-                                  }),
-                     "'must include' and 'must exclude' sets must be disjoint");
-
-  if (Bound == GlobalsLocalizationConfig::NoLimit) {
-    std::vector<GVRefWrapper> ToLocalize;
-    // filter out those, that we must exclude
-    std::copy_if(Globals.begin(), Globals.end(), std::back_inserter(ToLocalize),
-                 [ExcludePred](GVRef GV) { return !ExcludePred(GV); });
-    return ToLocalize;
-  }
-
-  std::vector<GVRefWrapper> ToLocalize;
-  // Adding those that we must include.
-  std::copy_if(Globals.begin(), Globals.end(), std::back_inserter(ToLocalize),
-               IncludePred);
-  if (Bound == 0)
-    return ToLocalize;
-
-  T IncludeWeight =
-      std::accumulate(ToLocalize.begin(), ToLocalize.end(), static_cast<T>(0),
-                      [WeightCalculator](T Prev, GVRef GV) {
-                        return Prev + WeightCalculator(GV);
-                      });
-  if (IncludeWeight >= Bound)
-    return ToLocalize;
-
-  std::vector<GVRefWrapper> Remainder;
-  std::copy_if(Globals.begin(), Globals.end(), std::back_inserter(Remainder),
-               [IncludePred, ExcludePred](GVRef GV) {
-                 return !IncludePred(GV) && !ExcludePred(GV);
-               });
-  // Sorting remaining globals by weight.
-  std::sort(Remainder.begin(), Remainder.end(),
-            [WeightCalculator](GVRef LHS, GVRef RHS) {
-              return WeightCalculator(LHS) < WeightCalculator(RHS);
-            });
-
-  T RemainderBound = Bound - IncludeWeight;
-  // filter max number of lightest ones, which weight sum is under the bound
-  auto FirstNotToLocalize = vc::upper_partial_sum_bound(
-      Remainder.begin(), Remainder.end(), RemainderBound,
-      [WeightCalculator](T Base, GVRef Inc) {
-        return Base + WeightCalculator(Inc);
-      });
-
-  std::copy(Remainder.begin(), FirstNotToLocalize,
-            std::back_inserter(ToLocalize));
-  return ToLocalize;
-}
-
 bool CMABIAnalysis::runOnCallGraph(CallGraph &CG) {
   // Analyze global variable usages and for each function attaches global
   // variables to be copy-in and copy-out.
@@ -541,45 +435,25 @@ bool CMABI::runOnSCC(CallGraphSCC &SCC) {
   return Changed;
 }
 
-// Whether \p Inst is an instruction on which IR rebuild caused by addrspace
-// change will stop.
-static bool isRebuildTerminal(const Instruction &Inst) {
-  // Result of a load inst is no longer a pointer so here propogation will stop.
-  if (isa<LoadInst>(Inst) || isa<AddrSpaceCastInst>(Inst) ||
-      isa<StoreInst>(Inst))
-    return true;
-  if (!isa<IntrinsicInst>(Inst))
-    return false;
-  auto IID = cast<IntrinsicInst>(Inst).getIntrinsicID();
-  return IID == Intrinsic::masked_gather || IID == Intrinsic::masked_scatter;
-}
-
 // Replaces uses of global variables with the corresponding allocas inside a
 // specified function. More insts can be rebuild if global variable addrspace
 // wasn't private.
 static void replaceUsesWithinFunction(
     const SmallDenseMap<Value *, Value *> &GlobalsToReplace, Function *F) {
-  auto ToRebuild = vc::MakeRebuildInfoBuilder(
-      [](const Instruction &Inst) { return isRebuildTerminal(Inst); });
-  ReversePostOrderTraversal<Function *> RPOT(F);
-  for (auto *BB : RPOT) {
-    for (auto &Inst : *BB) {
+  for (auto &BB : *F) {
+    for (auto &Inst : BB) {
       for (unsigned i = 0, e = Inst.getNumOperands(); i < e; ++i) {
         Value *Op = Inst.getOperand(i);
         auto Iter = GlobalsToReplace.find(Op);
         if (Iter != GlobalsToReplace.end()) {
-          if (Op->getType() == Iter->second->getType())
-            Inst.setOperand(i, Iter->second);
-          else {
-            ToRebuild.addEntry(Inst, i, *Iter->second);
-          }
-        } else {
-          ToRebuild.addNodeIfRequired(Inst, i);
+          IGC_ASSERT_MESSAGE(Op->getType() == Iter->second->getType(),
+                             "only global variables in private addrspace are "
+                             "localized, so types must match");
+          Inst.setOperand(i, Iter->second);
         }
       }
     }
   }
-  vc::MakeInstructionRebuilder(std::move(ToRebuild).emit()).rebuild();
 }
 
 // \brief Create allocas for globals directly used in this kernel and
@@ -1534,21 +1408,15 @@ void CMABIAnalysis::analyzeGlobals(CallGraph &CG) {
   if (M.global_empty())
     return;
 
-  const auto &DL = M.getDataLayout();
-  auto ToLocalize = selectGlobalsToLocalize(
-      M.globals(), GlobalsLocalizationLimit,
-      [](const GlobalVariable &GV) {
-        // Don't localize global constant format string, as it must be
-        // relocated in case of zebin printf.
-        // FIXME: what if we force localization.
-        return GV.hasAttribute(genx::FunctionMD::GenXVolatile) ||
-               vc::isConstantString(GV);
-      },
-      [IncludeVectors = LocalizeVectorGlobals](const GlobalVariable &GV) {
-        return IncludeVectors && GV.getValueType()->isVectorTy() &&
-               !GV.hasAttribute(genx::FunctionMD::GenXVolatile);
-      },
-      [&DL](const GlobalVariable &GV) { return calcGVWeight(GV, DL); });
+  // FIXME: String constants must be localized too. Excluding them there
+  //        to WA legacy printf implementation in CM FE (printf strings are
+  //        not in constant addrspace in legacy printf).
+  auto ToLocalize =
+      make_filter_range(M.globals(), [](const GlobalVariable &GV) {
+        return GV.getAddressSpace() == PrivateAddrSpace &&
+               !GV.hasAttribute(genx::FunctionMD::GenXVolatile) &&
+               !vc::isConstantString(GV);
+      });
 
   // Collect direct and indirect (GV is used in a called function)
   // uses of globals.
diff --git a/IGC/VectorCompiler/lib/Support/BackendConfig.cpp b/IGC/VectorCompiler/lib/Support/BackendConfig.cpp
