[InstrProfiling] Mark __llvm_profile_instrument_target last parameter as i32 zeroext or signext if appropriate.

[• koriakin]

Summary

On some architectures (s390x, ppc64, sparc64, mips), C-level int is passed
as i32 signext instead of plain i32. Likewise, unsigned int may be passed
as i32, i32 signext, or i32 zeroext depending on the platform. Mark
__llvm_profile_instrument_target properly (its last parameter is unsigned
int).

This (together with the clang change) makes compiler-rt profile testsuite pass
on s390x.

[vsk]

@davidxl would be a good reviewer for this.

include/llvm/Analysis/TargetLibraryInfo.h

178

Could you split this up into two setters, to better match the getters below?

include/llvm/Transforms/InstrProfiling.h

35

It appears that TLI is assumed to be non-null. Can you make this a reference?

lib/Transforms/Instrumentation/InstrProfiling.cpp

177

Unrelated to this patch, but: it seems weird that this is called every time we do a lowerValueProfileInst(). Can we just do it once per run?

[vsk]

Also, this is missing a test.

[• koriakin]

In D21736#467527, @vsk wrote:

Also, this is missing a test.

True. I'm going to add a clang test in a separate patch (this way it will cover D21737 as well). Should I make an llvm test as well?

[vsk]

In D21736#467529, @koriakin wrote:

In D21736#467527, @vsk wrote:

Also, this is missing a test.

True. I'm going to add a clang test in a separate patch (this way it will cover D21737 as well). Should I make an llvm test as well?

Yes, that would be helpful.

[davidxl]

The TLI change is independent of InstProfile changes. Please split the InstProf change into a separate patch.

[• koriakin]

Split off the TLI part, changed pointer to reference.

[• koriakin]

Added a test.

[vsk]

LGTM.

[davidxl]

One nit. Can you save TLI pointer in InstProfiling instead of passing it around?

[• koriakin]

In D21736#467986, @davidxl wrote:

One nit. Can you save TLI pointer in InstProfiling instead of passing it around?

Save where? I thought run is not supposed to modify the Pass object, and I don't have TLI before that.

[• koriakin]

Saved TLI as a field of InstProfiling.

[davidxl]

lgtm

[• koriakin]

Changed to use TargetTransformInfo, per comment on D21739. D21737 is now no longer necessary.

I'm not sure if keeping TTI on the pass object is a good idea now...

[• koriakin]

Added another case for mips, which uses i32 signext.

[• koriakin]

Split off the tests (they'll come together with target-specific implementations for D21739).

[• koriakin]

Updated for new revision of D21739 - now aborts if knownTarget() is false.

[• koriakin]

Moved the functions to TLI, as per ML: http://lists.llvm.org/pipermail/llvm-dev/2016-September/104641.html.

[• koriakin]

It seems I have accidentally swapped the diffs of this revision and D21739. Here comes the correct diff.

[• koriakin]

Readded missing test.

[• koriakin]

Changed to composite getExtAttrForI32Param/Return functions as for D21739.

[hfinkel]

LGTM too.

include/llvm/Analysis/TargetLibraryInfo.h

 /// depends on the triple. So users typically interact with the \c
 /// TargetLibraryInfo wrapper below.
 class TargetLibraryInfoImpl {
   friend class TargetLibraryInfo;
 
   unsigned char AvailableArray[(LibFunc::NumLibFuncs+3)/4];
   llvm::DenseMap<unsigned, std::string> CustomNames;
   static const char *const StandardNames[LibFunc::NumLibFuncs];
+  bool ShouldExtI32Param, ShouldExtI32Return;
 
   enum AvailabilityState {
     StandardName = 3, // (memset to all ones)
     CustomName = 1,
     Unavailable = 0  // (memset to all zeros)
   };
   void setState(LibFunc::Func F, AvailabilityState State) {
     AvailableArray[F/4] &= ~(3 << 2*(F&3));
bool isFunctionScalarizable(StringRef F, unsigned &VF) const {
     return !getScalarizedFunction(F, VF).empty();
   }
 
   /// Return the name of the equivalent of F, scalarized. If no such mapping
   /// exists, return the empty string.
   ///
   /// Set VF to the vectorization factor.
   StringRef getScalarizedFunction(StringRef F, unsigned &VF) const;
+
+  /// Set whether int and unsigned int parameters and returns should have
+  ///  signext or zeroext attributes, respectively.
+  void setShouldExtI32(bool Param, bool Return) {
+    ShouldExtI32Param = Param;
+    ShouldExtI32Return = Return;
+  }
 };
 
 /// Provides information about what library functions are available for
 /// the current target.
 ///
 /// This both allows optimizations to handle them specially and frontends to
 /// disable such optimizations through -fno-builtin etc.
 class TargetLibraryInfo {
StringRef getName(LibFunc::Func F) const {
     if (State == TargetLibraryInfoImpl::Unavailable)
       return StringRef();
     if (State == TargetLibraryInfoImpl::StandardName)
       return Impl->StandardNames[F];
     assert(State == TargetLibraryInfoImpl::CustomName);
     return Impl->CustomNames.find(F)->second;
   }
 
+  /// Returns true iff i32 parameters to library functions should have
+  /// signext or zeroext attributes if they correspond to C-level int or
+  /// unsigned int, respectively.
+  bool shouldExtI32Param() const {
+    return Impl->ShouldExtI32Param;
+  }
+
+  /// Returns true iff i32 returns from library functions should have
+  /// signext or zeroext attributes if they correspond to C-level int or
+  /// unsigned int, respectively.
+  bool shouldExtI32Return() const {
+    return Impl->ShouldExtI32Return;
+  }
+
   /// Handle invalidation from the pass manager.
   ///
   /// If we try to invalidate this info, just return false. It cannot become
   /// invalid even if the module changes.
   bool invalidate(Module &, const PreservedAnalyses &) { return false; }
 };
 
 /// Analysis pass providing the \c TargetLibraryInfo.

include/llvm/Transforms/InstrProfiling.h

 
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/ProfileData/InstrProf.h"
 #include "llvm/Transforms/Instrumentation.h"
 
 namespace llvm {
 
+class TargetLibraryInfo;
+
 /// Instrumenation based profiling lowering pass. This pass lowers
 /// the profile instrumented code generated by FE or the IR based
 /// instrumentation pass.
 class InstrProfiling : public PassInfoMixin<InstrProfiling> {
 public:
   InstrProfiling() {}
   InstrProfiling(const InstrProfOptions &Options) : Options(Options) {}
 
   PreservedAnalyses run(Module &M, AnalysisManager<Module> &AM);
-  bool run(Module &M);
+  bool run(Module &M, const TargetLibraryInfo *TLI);
 
 private:
   InstrProfOptions Options;
   Module *M;
   struct PerFunctionProfileData {
     uint32_t NumValueSites[IPVK_Last + 1];
     GlobalVariable *RegionCounters;
     GlobalVariable *DataVar;
private:
 
   /// Get the section name for the coverage mapping data.
   StringRef getCoverageSection() const;
 
   /// Count the number of instrumented value sites for the function.
   void computeNumValueSiteCounts(InstrProfValueProfileInst *Ins);
 
   /// Replace instrprof_value_profile with a call to runtime library.
-  void lowerValueProfileInst(InstrProfValueProfileInst *Ins);
+  void lowerValueProfileInst(InstrProfValueProfileInst *Ins,
+                             const TargetLibraryInfo *TLI);
 
   /// Replace instrprof_increment with an increment of the appropriate value.
   void lowerIncrement(InstrProfIncrementInst *Inc);
 
   /// Force emitting of name vars for unused functions.
   void lowerCoverageData(GlobalVariable *CoverageNamesVar);
 
   /// Get the region counters for an increment, creating them if necessary.

lib/Analysis/TargetLibraryInfo.cpp

static void initialize(TargetLibraryInfoImpl &TLI, const Triple &T,
   if (T.isNVPTX()) {
     TLI.disableAllFunctions();
     TLI.setAvailable(LibFunc::nvvm_reflect);
   } else {
     TLI.setUnavailable(LibFunc::nvvm_reflect);
   }
 
   TLI.addVectorizableFunctionsFromVecLib(ClVectorLibrary);
+
+  // PowerPC64, Sparc64, SystemZ need signext/zeroext on i32 parameters and
+  // returns corresponding to C-level ints.  Mips64 needs it for parameters
+  // only.
+  bool ShouldExtI32Param = false, ShouldExtI32Return = false;
+  if (T.getArch() == Triple::ppc64 || T.getArch() == Triple::ppc64le ||
+      T.getArch() == Triple::sparcv9 || T.getArch() == Triple::systemz) {
+    ShouldExtI32Param = true;
+    ShouldExtI32Return = true;
+  }
+  if (T.getArch() == Triple::mips64 || T.getArch() == Triple::mips64el)
+    ShouldExtI32Param = true;
+  TLI.setShouldExtI32(ShouldExtI32Param, ShouldExtI32Return);
 }
 
 TargetLibraryInfoImpl::TargetLibraryInfoImpl() {
   // Default to everything being available.
   memset(AvailableArray, -1, sizeof(AvailableArray));
 
   initialize(*this, Triple(), StandardNames);
 }
 
 TargetLibraryInfoImpl::TargetLibraryInfoImpl(const Triple &T) {
   // Default to everything being available.
   memset(AvailableArray, -1, sizeof(AvailableArray));
 
   initialize(*this, T, StandardNames);
 }
 
 TargetLibraryInfoImpl::TargetLibraryInfoImpl(const TargetLibraryInfoImpl &TLI)
-    : CustomNames(TLI.CustomNames) {
+    : CustomNames(TLI.CustomNames), ShouldExtI32Param(TLI.ShouldExtI32Param),
+      ShouldExtI32Return(TLI.ShouldExtI32Return) {
   memcpy(AvailableArray, TLI.AvailableArray, sizeof(AvailableArray));
   VectorDescs = TLI.VectorDescs;
   ScalarDescs = TLI.ScalarDescs;
 }
 
 TargetLibraryInfoImpl::TargetLibraryInfoImpl(TargetLibraryInfoImpl &&TLI)
-    : CustomNames(std::move(TLI.CustomNames)) {
+    : CustomNames(std::move(TLI.CustomNames)),
+      ShouldExtI32Param(TLI.ShouldExtI32Param),
+      ShouldExtI32Return(TLI.ShouldExtI32Return) {
   std::move(std::begin(TLI.AvailableArray), std::end(TLI.AvailableArray),
             AvailableArray);
   VectorDescs = TLI.VectorDescs;
   ScalarDescs = TLI.ScalarDescs;
 }
 
 TargetLibraryInfoImpl &TargetLibraryInfoImpl::operator=(const TargetLibraryInfoImpl &TLI) {
   CustomNames = TLI.CustomNames;
+  ShouldExtI32Param = TLI.ShouldExtI32Param;
+  ShouldExtI32Return = TLI.ShouldExtI32Return;
   memcpy(AvailableArray, TLI.AvailableArray, sizeof(AvailableArray));
   return *this;
 }
 
 TargetLibraryInfoImpl &TargetLibraryInfoImpl::operator=(TargetLibraryInfoImpl &&TLI) {
   CustomNames = std::move(TLI.CustomNames);
+  ShouldExtI32Param = TLI.ShouldExtI32Param;
+  ShouldExtI32Return = TLI.ShouldExtI32Return;
   std::move(std::begin(TLI.AvailableArray), std::end(TLI.AvailableArray),
             AvailableArray);
   return *this;
 }
 
 static StringRef sanitizeFunctionName(StringRef funcName) {
   // Filter out empty names and names containing null bytes, those can't be in
   // our table.

lib/Transforms/Instrumentation/InstrProfiling.cpp

 // This pass lowers instrprof_* intrinsics emitted by a frontend for profiling.
 // It also builds the data structures and initialization code needed for
 // updating execution counts and emitting the profile at runtime.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Transforms/InstrProfiling.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Module.h"
 #include "llvm/ProfileData/InstrProf.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 
 using namespace llvm;
 
public:
   static char ID;
   InstrProfilingLegacyPass() : ModulePass(ID), InstrProf() {}
   InstrProfilingLegacyPass(const InstrProfOptions &Options)
       : ModulePass(ID), InstrProf(Options) {}
   const char *getPassName() const override {
     return "Frontend instrumentation-based coverage lowering";
   }
 
-  bool runOnModule(Module &M) override { return InstrProf.run(M); }
+  bool runOnModule(Module &M) override {
+    return InstrProf.run(M, &getAnalysis<TargetLibraryInfoWrapperPass>().getTLI());
+  }
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.setPreservesCFG();
+    AU.addRequired<TargetLibraryInfoWrapperPass>();
   }
 };
 
 } // anonymous namespace
 
 PreservedAnalyses InstrProfiling::run(Module &M, AnalysisManager<Module> &AM) {
-  if (!run(M))
+  auto TLI = &AM.getResult<TargetLibraryAnalysis>(M);
+  if (!run(M, TLI))
     return PreservedAnalyses::all();
 
   return PreservedAnalyses::none();
 }
 
 char InstrProfilingLegacyPass::ID = 0;
-INITIALIZE_PASS(InstrProfilingLegacyPass, "instrprof",
-                "Frontend instrumentation-based coverage lowering.", false,
-                false)
+INITIALIZE_PASS_BEGIN(
+    InstrProfilingLegacyPass, "instrprof",
+    "Frontend instrumentation-based coverage lowering.", false, false)
+INITIALIZE_PASS_DEPENDENCY(TargetLibraryInfoWrapperPass)
+INITIALIZE_PASS_END(
+    InstrProfilingLegacyPass, "instrprof",
+    "Frontend instrumentation-based coverage lowering.", false, false)
 
 ModulePass *
 llvm::createInstrProfilingLegacyPass(const InstrProfOptions &Options) {
   return new InstrProfilingLegacyPass(Options);
 }
 
 bool InstrProfiling::isMachO() const {
   return Triple(M->getTargetTriple()).isOSBinFormatMachO();
StringRef InstrProfiling::getDataSection() const {
   return getInstrProfDataSectionName(isMachO());
 }
 
 /// Get the section name for the coverage mapping data.
 StringRef InstrProfiling::getCoverageSection() const {
   return getInstrProfCoverageSectionName(isMachO());
 }
 
-bool InstrProfiling::run(Module &M) {
+bool InstrProfiling::run(Module &M, const TargetLibraryInfo *TLI) {
   bool MadeChange = false;
 
   this->M = &M;
   NamesVar = nullptr;
   NamesSize = 0;
   ProfileDataMap.clear();
   UsedVars.clear();
 
bool InstrProfiling::run(Module &M, const TargetLibraryInfo *TLI) {
   for (Function &F : M)
     for (BasicBlock &BB : F)
       for (auto I = BB.begin(), E = BB.end(); I != E;) {
         auto Instr = I++;
         if (auto *Inc = dyn_cast<InstrProfIncrementInst>(Instr)) {
           lowerIncrement(Inc);
           MadeChange = true;
         } else if (auto *Ind = dyn_cast<InstrProfValueProfileInst>(Instr)) {
-          lowerValueProfileInst(Ind);
+          lowerValueProfileInst(Ind, TLI);
           MadeChange = true;
         }
       }
 
   if (GlobalVariable *CoverageNamesVar =
           M.getNamedGlobal(getCoverageUnusedNamesVarName())) {
     lowerCoverageData(CoverageNamesVar);
     MadeChange = true;
   }
 
   if (!MadeChange)
     return false;
 
   emitVNodes();
   emitNameData();
   emitRegistration();
   emitRuntimeHook();
   emitUses();
   emitInitialization();
   return true;
 }
 
-static Constant *getOrInsertValueProfilingCall(Module &M) {
+static Constant *getOrInsertValueProfilingCall(Module &M,
+                                               const TargetLibraryInfo *TLI) {
   LLVMContext &Ctx = M.getContext();
   auto *ReturnTy = Type::getVoidTy(M.getContext());
   Type *ParamTypes[] = {
 #define VALUE_PROF_FUNC_PARAM(ParamType, ParamName, ParamLLVMType) ParamLLVMType
 #include "llvm/ProfileData/InstrProfData.inc"
   };
   auto *ValueProfilingCallTy =
       FunctionType::get(ReturnTy, makeArrayRef(ParamTypes), false);
-  return M.getOrInsertFunction(getInstrProfValueProfFuncName(),
+  Constant *Res = M.getOrInsertFunction(getInstrProfValueProfFuncName(),
                                         ValueProfilingCallTy);
+  Function *FunRes = dyn_cast<Function>(Res);
+  if (TLI->shouldExtI32Param() && FunRes)
+    FunRes->addAttribute(3, Attribute::ZExt);
+  return Res;
 }
 
 void InstrProfiling::computeNumValueSiteCounts(InstrProfValueProfileInst *Ind) {
 
   GlobalVariable *Name = Ind->getName();
   uint64_t ValueKind = Ind->getValueKind()->getZExtValue();
   uint64_t Index = Ind->getIndex()->getZExtValue();
   auto It = ProfileDataMap.find(Name);
   if (It == ProfileDataMap.end()) {
     PerFunctionProfileData PD;
     PD.NumValueSites[ValueKind] = Index + 1;
     ProfileDataMap[Name] = PD;
   } else if (It->second.NumValueSites[ValueKind] <= Index)
     It->second.NumValueSites[ValueKind] = Index + 1;
 }
 
-void InstrProfiling::lowerValueProfileInst(InstrProfValueProfileInst *Ind) {
+void InstrProfiling::lowerValueProfileInst(InstrProfValueProfileInst *Ind,
+                                           const TargetLibraryInfo *TLI) {
 
   GlobalVariable *Name = Ind->getName();
   auto It = ProfileDataMap.find(Name);
   assert(It != ProfileDataMap.end() && It->second.DataVar &&
          "value profiling detected in function with no counter incerement");
 
   GlobalVariable *DataVar = It->second.DataVar;
   uint64_t ValueKind = Ind->getValueKind()->getZExtValue();
   uint64_t Index = Ind->getIndex()->getZExtValue();
   for (uint32_t Kind = IPVK_First; Kind < ValueKind; ++Kind)
     Index += It->second.NumValueSites[Kind];
 
   IRBuilder<> Builder(Ind);
   Value *Args[3] = {Ind->getTargetValue(),
                     Builder.CreateBitCast(DataVar, Builder.getInt8PtrTy()),
                     Builder.getInt32(Index)};
-  Ind->replaceAllUsesWith(
-      Builder.CreateCall(getOrInsertValueProfilingCall(*M), Args));
+  CallInst *Call = Builder.CreateCall(getOrInsertValueProfilingCall(*M, TLI),
+                                      Args);
+  if (TLI->shouldExtI32Param())
+    Call->addAttribute(3, Attribute::ZExt);
+  Ind->replaceAllUsesWith(Call);
   Ind->eraseFromParent();
 }
 
 void InstrProfiling::lowerIncrement(InstrProfIncrementInst *Inc) {
   GlobalVariable *Counters = getOrCreateRegionCounters(Inc);
 
   IRBuilder<> Builder(Inc);
   uint64_t Index = Inc->getIndex()->getZExtValue();