[llvm-branch-commits] [NFC][memprof] Use %p to print addresses (PR #98577)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson approved this pull request.


https://github.com/llvm/llvm-project/pull/98577
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [misexpect] Support diagnostics from frontend profile data (PR #96524)

[Teresa Johnson via llvm-branch-commits]

@@ -369,9 +369,21 @@ static bool lowerExpectIntrinsic(Function &F) {
 if (BranchInst *BI = dyn_cast(BB.getTerminator())) {
   if (handleBranchExpect(*BI))
 ExpectIntrinsicsHandled++;
+  else {
+SmallVector Weights;
+if (extractBranchWeights(*BI, Weights))
+  misexpect::checkMissingAnnotations(*BI, Weights,
+ /*IsFrontendInstr=*/false);

teresajohnson wrote:

Confused about how this relates to enabling checking for frontend 
instrumentation since all of these set that flag to false?

https://github.com/llvm/llvm-project/pull/96524
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [misexpect] Support diagnostics from frontend profile data (PR #96524)

[Teresa Johnson via llvm-branch-commits]

@@ -3,7 +3,7 @@
 
 ; RUN: llvm-profdata merge %S/Inputs/misexpect-branch-correct.proftext -o 
%t.profdata
 
-; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" 
-pgo-test-profile-file=%t.profdata -pgo-missing-annotations 
-pass-remarks=missing-annotation -S 2>&1 | FileCheck %s 
--check-prefix=MISSING_ANNOTATION
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" 
-pgo-test-profile-file=%t.profdata -pgo-missing-annotations 
-pass-remarks=missing-annotations -S 2>&1 | FileCheck %s 
--check-prefix=MISSING_ANNOTATION

teresajohnson wrote:

Can you clarify how this change relates to the new functionality in this PR?

Also, the description of this test seems wrong? There are also some "CHECK" 
labels further below that don't have a corresponding FileCheck invocation. 
Looks like this might be leftover from a different test this is based off of.

https://github.com/llvm/llvm-project/pull/96524
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [NFC][IndirectCallProm] Refactor function-based conditional devirtualization and indirect call value profile update into one helper function (PR #80762)

[Teresa Johnson via llvm-branch-commits]

@@ -1260,6 +1260,8 @@ void annotateValueSite(Module &M, Instruction &Inst,
ArrayRef VDs,
uint64_t Sum, InstrProfValueKind ValueKind,
uint32_t MaxMDCount) {
+  if (VDs.empty())

teresajohnson wrote:

Oh, is this because you no longer check the "NumPromoted == NumVals" case?

https://github.com/llvm/llvm-project/pull/80762
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [NFC][IndirectCallProm] Refactor function-based conditional devirtualization and indirect call value profile update into one helper function (PR #80762)

[Teresa Johnson via llvm-branch-commits]

@@ -1260,6 +1260,8 @@ void annotateValueSite(Module &M, Instruction &Inst,
ArrayRef VDs,
uint64_t Sum, InstrProfValueKind ValueKind,
uint32_t MaxMDCount) {
+  if (VDs.empty())

teresajohnson wrote:

Why this change?

https://github.com/llvm/llvm-project/pull/80762
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [NFC][IndirectCallProm] Refactor function-based conditional devirtualization and indirect call value profile update into one helper function (PR #80762)

[Teresa Johnson via llvm-branch-commits]

@@ -287,7 +290,18 @@ uint32_t IndirectCallPromoter::tryToPromote(
 NumOfPGOICallPromotion++;
 NumPromoted++;
   }
-  return NumPromoted;
+
+  const bool Changed = (NumPromoted != 0);

teresajohnson wrote:

Just return false early if NumPromoted is 0 (then Changed can be removed and 
just return true at the end further below).

https://github.com/llvm/llvm-project/pull/80762
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [NFC]Extract the heuristic to find vtable for an indirect call into a helper function (PR #81024)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson approved this pull request.


https://github.com/llvm/llvm-project/pull/81024
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [NFC] (PR #80762)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson approved this pull request.


https://github.com/llvm/llvm-project/pull/80762
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [InstrProf] Add vtables with type metadata into symtab to look it up with GUID (PR #81051)

[Teresa Johnson via llvm-branch-commits]

teresajohnson wrote:

> Whoops! Sorry! Please don't review this.
> 
> Something goes wrong with my fixup.

Let me know when it is ready for review.

https://github.com/llvm/llvm-project/pull/81051
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [InstrProf] Add vtables with type metadata into symtab to look it up with GUID (PR #81051)

[Teresa Johnson via llvm-branch-commits]

teresajohnson wrote:

Can you pull out the part that does the refactoring into getCanonicalName and 
adds comments etc into a separate nfc patch, so that this one is just about 
adding vtables?

https://github.com/llvm/llvm-project/pull/81051
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [NFC][CallPromotionUtils]Extract a helper function versionCallSiteWithCond from versionCallSite (PR #81181)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson approved this pull request.

lgtm with a suggested change to the comments

https://github.com/llvm/llvm-project/pull/81181
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [NFC][CallPromotionUtils]Extract a helper function versionCallSiteWithCond from versionCallSite (PR #81181)

[Teresa Johnson via llvm-branch-commits]

@@ -267,7 +264,6 @@ static void createRetBitCast(CallBase &CB, Type *RetTy, 
CastInst **RetBitCast) {
 /// Is replaced by the following:
 ///
 ///   cond_bb:
-/// %cond = icmp eq i32 ()* %ptr, @func

teresajohnson wrote:

I'm not sure these conditions should be completely removed. Perhaps just show 
something like `%cond = Cond`.

https://github.com/llvm/llvm-project/pull/81181
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [NFC][CallPromotionUtils]Extract a helper function versionCallSiteWithCond from versionCallSite (PR #81181)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson edited 
https://github.com/llvm/llvm-project/pull/81181
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [InstrProf] Add vtables with type metadata into symtab to look it up with GUID (PR #81051)

[Teresa Johnson via llvm-branch-commits]

@@ -479,15 +479,37 @@ Error InstrProfSymtab::create(Module &M, bool InLTO) {
   continue;
 Types.clear();
 G.getMetadata(LLVMContext::MD_type, Types);
-if (!Types.empty()) {
-  MD5VTableMap.emplace_back(G.getGUID(), &G);
-}
+if (Types.empty())
+  continue;
+if (Error E = addVTableWithName(
+  G, getIRPGOObjectName(G, InLTO, /* PGONameMetadata */ nullptr)))
+return E;
   }
   Sorted = false;
   finalizeSymtab();
   return Error::success();
 }
 
+Error InstrProfSymtab::addVTableWithName(GlobalVariable &VTable,
+ StringRef VTablePGOName) {
+  
+  auto mapName = [&](StringRef Name) -> Error {
+if (Error E = addVTableName(Name))

teresajohnson wrote:

Where is addVTableName defined?

https://github.com/llvm/llvm-project/pull/81051
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [nfc][InstrProfiling]For comdat setting helper function, move comment closer to the code (PR #83757)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson approved this pull request.


https://github.com/llvm/llvm-project/pull/83757
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] release/18.x: [clang][fat-lto-objects] Make module flags match non-FatLTO pipelines (#83159) (PR #84290)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson approved this pull request.


https://github.com/llvm/llvm-project/pull/84290
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [ThinLTO] Generate import status in per-module combined summary (PR #88024)

[Teresa Johnson via llvm-branch-commits]

@@ -1634,17 +1752,27 @@ Expected FunctionImporter::importFunctions(
   return std::move(Err);
 
 auto &ImportGUIDs = FunctionsToImportPerModule->second;
+
 // Find the globals to import
 SetVector GlobalsToImport;
 for (Function &F : *SrcModule) {
   if (!F.hasName())
 continue;
   auto GUID = F.getGUID();
-  auto Import = ImportGUIDs.count(GUID);
-  LLVM_DEBUG(dbgs() << (Import ? "Is" : "Not") << " importing function "
-<< GUID << " " << F.getName() << " from "
-<< SrcModule->getSourceFileName() << "\n");
-  if (Import) {
+  auto ImportType = maybeGetImportType(ImportGUIDs, GUID);
+
+  if (!ImportType) {

teresajohnson wrote:

Also consider indicating which are imported as declarations in the debug 
message?

https://github.com/llvm/llvm-project/pull/88024
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [ThinLTO] Generate import status in per-module combined summary (PR #88024)

[Teresa Johnson via llvm-branch-commits]

@@ -158,7 +158,7 @@ void llvm::computeLTOCacheKey(
 
   std::vector ExportsGUID;
   ExportsGUID.reserve(ExportList.size());
-  for (const auto &VI : ExportList) {
+  for (const auto &[VI, UnusedImportType] : ExportList) {

teresajohnson wrote:

We should probably include the new import type result in the cache key. Because 
if that changes then presumably the cached object should be invalidated as it 
would be different?

https://github.com/llvm/llvm-project/pull/88024
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [ThinLTO] Generate import status in per-module combined summary (PR #88024)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson edited 
https://github.com/llvm/llvm-project/pull/88024
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [ThinLTO] Generate import status in per-module combined summary (PR #88024)

[Teresa Johnson via llvm-branch-commits]

@@ -1670,11 +1798,15 @@ Expected FunctionImporter::importFunctions(
   if (!GV.hasName())
 continue;
   auto GUID = GV.getGUID();
-  auto Import = ImportGUIDs.count(GUID);
-  LLVM_DEBUG(dbgs() << (Import ? "Is" : "Not") << " importing global "
-<< GUID << " " << GV.getName() << " from "
-<< SrcModule->getSourceFileName() << "\n");
-  if (Import) {
+  auto ImportType = maybeGetImportType(ImportGUIDs, GUID);
+  if (!ImportType)

teresajohnson wrote:

Do we need to emit a debug message in this case like you are doing for 
functions above? Ditto for aliases below

https://github.com/llvm/llvm-project/pull/88024
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [ThinLTO] Generate import status in per-module combined summary (PR #88024)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson commented:

I only had time for a cursory review, some comments / suggestions below. I also 
have a suggestion for the testing issue wrt to the other patch, will note that 
separately

https://github.com/llvm/llvm-project/pull/88024
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [ThinLTO] Generate import status in per-module combined summary (PR #88024)

[Teresa Johnson via llvm-branch-commits]

@@ -1634,17 +1752,27 @@ Expected FunctionImporter::importFunctions(
   return std::move(Err);
 
 auto &ImportGUIDs = FunctionsToImportPerModule->second;
+
 // Find the globals to import
 SetVector GlobalsToImport;
 for (Function &F : *SrcModule) {
   if (!F.hasName())
 continue;
   auto GUID = F.getGUID();
-  auto Import = ImportGUIDs.count(GUID);
-  LLVM_DEBUG(dbgs() << (Import ? "Is" : "Not") << " importing function "
-<< GUID << " " << F.getName() << " from "
-<< SrcModule->getSourceFileName() << "\n");
-  if (Import) {
+  auto ImportType = maybeGetImportType(ImportGUIDs, GUID);
+
+  if (!ImportType) {

teresajohnson wrote:

You could combine this with the below ImportDefinition checking to keep the 
same flow as before with one debug message, e.g.:

```
auto ImportType = maybeGetImportType(...);
auto ImportDefinition = false;
if (ImportType) {
   ImportDefinition = ...;
}
LLVM_DEBUG(dbgs() << (ImportDefinition ...
if (ImportDefinition) {
...
```

https://github.com/llvm/llvm-project/pull/88024
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [ThinLTO] Generate import status in per-module combined summary (PR #88024)

[Teresa Johnson via llvm-branch-commits]

@@ -245,8 +256,10 @@ static auto qualifyCalleeCandidates(
 }
 
 /// Given a list of possible callee implementation for a call site, select one
-/// that fits the \p Threshold. If none are found, the Reason will give the 
last
-/// reason for the failure (last, in the order of CalleeSummaryList entries).
+/// that fits the \p Threshold for function definition import. If none are
+/// found, the Reason will give the last reason for the failure (last, in the
+/// order of CalleeSummaryList entries). If caller wants to select eligible
+/// summary

teresajohnson wrote:

dangling sentence?

https://github.com/llvm/llvm-project/pull/88024
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [ThinLTO] Generate import status in per-module combined summary (PR #88024)

[Teresa Johnson via llvm-branch-commits]

@@ -1670,11 +1798,15 @@ Expected FunctionImporter::importFunctions(
   if (!GV.hasName())
 continue;
   auto GUID = GV.getGUID();
-  auto Import = ImportGUIDs.count(GUID);
-  LLVM_DEBUG(dbgs() << (Import ? "Is" : "Not") << " importing global "
-<< GUID << " " << GV.getName() << " from "
-<< SrcModule->getSourceFileName() << "\n");
-  if (Import) {
+  auto ImportType = maybeGetImportType(ImportGUIDs, GUID);
+  if (!ImportType)

teresajohnson wrote:

Or do what I suggested above which goes back to only needing one LLVM_DEBUG

https://github.com/llvm/llvm-project/pull/88024
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [ThinLTO] Generate import status in per-module combined summary (PR #88024)

[Teresa Johnson via llvm-branch-commits]

teresajohnson wrote:

> #87600 is a functional change and the diffbase of this patch, and 
> `llvm/test/ThinLTO/X86/import_callee_declaration.ll` should be a test case 
> for both patches.
> 
> In the [diffbase](https://github.com/llvm/llvm-project/pull/87600), bitcode 
> writer takes maps as additional parameters to populate import status, and 
> it's not straightforward to construct regression tests there without this 
> patch. I wonder if I shall introduce `cl::list` in 
> llvm-lto/llvm-lto2 (as a repeated arg) to specify `filename:GUID` to test the 
> diffbase alone.

Rather than add an option just for testing that one alone, I have a suggestion 
for splitting up the PRs slightly differently. What if you submitted this one 
first, minus the modified calls to writeIndexToFile and the part of the test 
that checks the disassembled index (just have the testing for this one check 
the number of declarations imported and other debug messages). Then move the 
modified calls to writeIndexToFile and the index disassembly checking to 
PR87600 that can be committed as a follow on? That way each change comes with a 
test.

https://github.com/llvm/llvm-project/pull/88024
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [ThinLTO][Bitcode] Generate import type in bitcode (PR #87600)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson approved this pull request.


https://github.com/llvm/llvm-project/pull/87600
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [compiler-rt] [ThinLTO][TypeProf] Implement vtable def import (PR #79381)

[Teresa Johnson via llvm-branch-commits]

@@ -124,6 +124,28 @@ static bool findRefEdges(ModuleSummaryIndex &Index, const 
User *CurUser,
 Worklist.push_back(Operand);
 }
   }
+
+  const Instruction *I = dyn_cast(CurUser);
+  if (I) {
+uint32_t ActualNumValueData = 0;
+uint64_t TotalCount = 0;
+// 24 is the maximum number of values preserved for one instrumented site,
+// defined by INSTR_PROF_DEFAULT_NUM_VAL_PER_SITE in
+// compiler-rt/lib/profile/InstrProfilingValue.c; passing 24 as
+// `MaxNumValueData` controls the max number of elements in the returned
+// array. The actual number of values is gated by the number of ops in 
!prof
+// metadata.
+auto ValueDataArray = getValueProfDataFromInst(
+*I, IPVK_VTableTarget, 24 /* MaxNumValueData */, ActualNumValueData,

teresajohnson wrote:

Do we really want 24? Or should we limit to something smaller (and tunable), 
like we do for virtual function pointer profiles in ICP.

https://github.com/llvm/llvm-project/pull/79381
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [compiler-rt] [ThinLTO][TypeProf] Implement vtable def import (PR #79381)

[Teresa Johnson via llvm-branch-commits]

@@ -129,15 +131,11 @@ static bool findRefEdges(ModuleSummaryIndex &Index, const 
User *CurUser,
   if (I) {
 uint32_t ActualNumValueData = 0;
 uint64_t TotalCount = 0;
-// 24 is the maximum number of values preserved for one instrumented site,
-// defined by INSTR_PROF_DEFAULT_NUM_VAL_PER_SITE in
-// compiler-rt/lib/profile/InstrProfilingValue.c; passing 24 as
-// `MaxNumValueData` controls the max number of elements in the returned
-// array. The actual number of values is gated by the number of ops in 
!prof
-// metadata.
+// MaxNumVTableAnnotations is the maximum number of vtables annotated on
+// the instruction.
 auto ValueDataArray = getValueProfDataFromInst(
-*I, IPVK_VTableTarget, 24 /* MaxNumValueData */, ActualNumValueData,
-TotalCount);
+*I, IPVK_VTableTarget, MaxNumVTableAnnotations /* MaxNumValueData */,

teresajohnson wrote:

I think you can remove the "/* MaxNumValueData */" now that this is not a 
constant. Or if you want to keep it use the format shown in my other comment.

https://github.com/llvm/llvm-project/pull/79381
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [compiler-rt] [ThinLTO][TypeProf] Implement vtable def import (PR #79381)

[Teresa Johnson via llvm-branch-commits]

@@ -124,6 +126,24 @@ static bool findRefEdges(ModuleSummaryIndex &Index, const 
User *CurUser,
 Worklist.push_back(Operand);
 }
   }
+
+  const Instruction *I = dyn_cast(CurUser);
+  if (I) {
+uint32_t ActualNumValueData = 0;
+uint64_t TotalCount = 0;
+// MaxNumVTableAnnotations is the maximum number of vtables annotated on
+// the instruction.
+auto ValueDataArray = getValueProfDataFromInst(
+*I, IPVK_VTableTarget, MaxNumVTableAnnotations /* MaxNumValueData */,
+ActualNumValueData, TotalCount);
+
+if (ValueDataArray.get()) {
+  for (uint32_t j = 0; j < ActualNumValueData; j++) {
+RefEdges.insert(Index.getOrInsertValueInfo(
+ValueDataArray[j].Value /* VTableGUID */));

teresajohnson wrote:

Use format "/* VTableGUID = */ ValueDataArray[j].Value" for consistency with 
other code.

https://github.com/llvm/llvm-project/pull/79381
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [compiler-rt] [ThinLTO][TypeProf] Implement vtable def import (PR #79381)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson approved this pull request.

lgtm with 2 minor comment nits

https://github.com/llvm/llvm-project/pull/79381
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [compiler-rt] [ThinLTO][TypeProf] Implement vtable def import (PR #79381)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson edited 
https://github.com/llvm/llvm-project/pull/79381
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] PR for llvm/llvm-project#79511 (PR #79513)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson approved this pull request.

sure seems fine

https://github.com/llvm/llvm-project/pull/79513
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm-branch] r277628 - Add ThinLTO release note.

[Teresa Johnson via llvm-branch-commits]

Author: tejohnson
Date: Wed Aug  3 13:52:04 2016
New Revision: 277628

URL: http://llvm.org/viewvc/llvm-project?rev=277628&view=rev
Log:
Add ThinLTO release note.

Modified:
llvm/branches/release_39/docs/ReleaseNotes.rst

Modified: llvm/branches/release_39/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/llvm/branches/release_39/docs/ReleaseNotes.rst?rev=277628&r1=277627&r2=277628&view=diff
==
--- llvm/branches/release_39/docs/ReleaseNotes.rst (original)
+++ llvm/branches/release_39/docs/ReleaseNotes.rst Wed Aug  3 13:52:04 2016
@@ -72,9 +72,6 @@ Non-comprehensive list of changes in thi
   need to be updated to replace the argument node and remove any dead nodes in
   cases where they currently return an ``SDNode *`` from this interface.
 
-* Introduction of ThinLTO: [FIXME: needs to be documented more extensively in
-  /docs/ ; ping Mehdi/Teresa before the release if not done]
-
 * Raised the minimum required CMake version to 3.4.3.
 
 .. NOTE
@@ -139,6 +136,16 @@ link-time may be differently optimized t
 during optimization, and may have arbitrarily different observable
 behavior.  See `PR26774 `_ for more details.
 
+Support for ThinLTO
+---
+
+LLVM now supports ThinLTO compilation, which can be invoked by compiling
+and linking with -flto=thin. The gold linker plugin, as well as linkers
+that use the new ThinLTO API in libLTO (like ld64), will transparently
+execute the ThinLTO backends in parallel threads.
+For more information on ThinLTO and the LLVM implementation, see the
+`ThinLTO blog post 
`_.
+
 Changes to the ARM Targets
 --
 


___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] 5b42fd8 - [LTO] Test format fix (NFC)

[Teresa Johnson via llvm-branch-commits]

Author: Teresa Johnson
Date: 2021-01-14T14:09:50-08:00
New Revision: 5b42fd8dd4e7e29125a09a41a33af7c9cb57d144

URL: 
https://github.com/llvm/llvm-project/commit/5b42fd8dd4e7e29125a09a41a33af7c9cb57d144
DIFF: 
https://github.com/llvm/llvm-project/commit/5b42fd8dd4e7e29125a09a41a33af7c9cb57d144.diff

LOG: [LTO] Test format fix (NFC)

As requested in D91583, use ';;' instead of ';' to preceed comments in
lld test. I did this in the equivalent gold test as well.

Added: 


Modified: 
lld/test/ELF/lto/devirt_vcall_vis_public.ll
llvm/test/tools/gold/X86/devirt_vcall_vis_public.ll

Removed: 




diff  --git a/lld/test/ELF/lto/devirt_vcall_vis_public.ll 
b/lld/test/ELF/lto/devirt_vcall_vis_public.ll
index 46e23537d4fa..3a8219a64ce8 100644
--- a/lld/test/ELF/lto/devirt_vcall_vis_public.ll
+++ b/lld/test/ELF/lto/devirt_vcall_vis_public.ll
@@ -1,24 +1,24 @@
 ; REQUIRES: x86
-; Test that --lto-whole-program-visibility enables devirtualization.
+;; Test that --lto-whole-program-visibility enables devirtualization.
 
-; Note that the --export-dynamic used below is simply to ensure symbols are
-; retained during linking.
+;; Note that the --export-dynamic used below is simply to ensure symbols are
+;; retained during linking.
 
-; Index based WPD
-; Generate unsplit module with summary for ThinLTO index-based WPD.
+;; Index based WPD
+;; Generate unsplit module with summary for ThinLTO index-based WPD.
 ; RUN: opt --thinlto-bc -o %t2.o %s
 ; RUN: ld.lld %t2.o -o %t3 -save-temps --lto-whole-program-visibility \
 ; RUN:  -mllvm -pass-remarks=. --export-dynamic 2>&1 | FileCheck %s 
--check-prefix=REMARK
 ; RUN: llvm-dis %t2.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
 
-; Hybrid WPD
-; Generate split module with summary for hybrid Thin/Regular LTO WPD.
+;; Hybrid WPD
+;; Generate split module with summary for hybrid Thin/Regular LTO WPD.
 ; RUN: opt --thinlto-bc --thinlto-split-lto-unit -o %t.o %s
 ; RUN: ld.lld %t.o -o %t3 -save-temps --lto-whole-program-visibility \
 ; RUN:  -mllvm -pass-remarks=. --export-dynamic 2>&1 | FileCheck %s 
--check-prefix=REMARK
 ; RUN: llvm-dis %t.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
 
-; Regular LTO WPD
+;; Regular LTO WPD
 ; RUN: opt -o %t4.o %s
 ; RUN: ld.lld %t4.o -o %t3 -save-temps --lto-whole-program-visibility \
 ; RUN:  -mllvm -pass-remarks=. --export-dynamic 2>&1 | FileCheck %s 
--check-prefix=REMARK
@@ -27,24 +27,24 @@
 ; REMARK-DAG: single-impl: devirtualized a call to _ZN1A1nEi
 ; REMARK-DAG: single-impl: devirtualized a call to _ZN1D1mEi
 
-; Try everything again but without -whole-program-visibility to confirm
-; WPD fails
+;; Try everything again but without -whole-program-visibility to confirm
+;; WPD fails
 
-; Index based WPD
+;; Index based WPD
 ; RUN: ld.lld %t2.o -o %t3 -save-temps \
 ; RUN:  -mllvm -pass-remarks=. --export-dynamic 2>&1 | FileCheck %s 
--implicit-check-not single-impl --allow-empty
 ; RUN: llvm-dis %t2.o.4.opt.bc -o - | FileCheck %s 
--check-prefix=CHECK-NODEVIRT-IR
-; Ensure --no-lto-whole-program-visibility overrides explicit 
--lto-whole-program-visibility.
+;; Ensure --no-lto-whole-program-visibility overrides explicit 
--lto-whole-program-visibility.
 ; RUN: ld.lld %t2.o -o %t3 -save-temps --lto-whole-program-visibility 
--no-lto-whole-program-visibility \
 ; RUN:  -mllvm -pass-remarks=. --export-dynamic 2>&1 | FileCheck %s 
--implicit-check-not single-impl --allow-empty
 ; RUN: llvm-dis %t2.o.4.opt.bc -o - | FileCheck %s 
--check-prefix=CHECK-NODEVIRT-IR
 
-; Hybrid WPD
+;; Hybrid WPD
 ; RUN: ld.lld %t.o -o %t3 -save-temps \
 ; RUN:  -mllvm -pass-remarks=. --export-dynamic 2>&1 | FileCheck %s 
--implicit-check-not single-impl --allow-empty
 ; RUN: llvm-dis %t.o.4.opt.bc -o - | FileCheck %s 
--check-prefix=CHECK-NODEVIRT-IR
 
-; Regular LTO WPD
+;; Regular LTO WPD
 ; RUN: ld.lld %t4.o -o %t3 -save-temps \
 ; RUN:  -mllvm -pass-remarks=. --export-dynamic 2>&1 | FileCheck %s 
--implicit-check-not single-impl --allow-empty
 ; RUN: llvm-dis %t3.0.4.opt.bc -o - | FileCheck %s 
--check-prefix=CHECK-NODEVIRT-IR
@@ -74,7 +74,7 @@ entry:
   %2 = bitcast i8** %fptrptr to i32 (%struct.A*, i32)**
   %fptr1 = load i32 (%struct.A*, i32)*, i32 (%struct.A*, i32)** %2, align 8
 
-  ; Check that the call was devirtualized.
+  ;; Check that the call was devirtualized.
   ; CHECK-IR: %call = tail call i32 @_ZN1A1nEi
   ; CHECK-NODEVIRT-IR: %call = tail call i32 %fptr1
   %call = tail call i32 %fptr1(%struct.A* nonnull %obj, i32 %a)
@@ -82,7 +82,7 @@ entry:
   %3 = bitcast i8** %vtable to i32 (%struct.A*, i32)**
   %fptr22 = load i32 (%struct.A*, i32)*, i32 (%struct.A*, i32)** %3, align 8
 
-  ; We still have to call it as virtual.
+  ;; We still have to call it as virtual.
   ; CHECK-IR: %call3 = tail call i32 %fptr22
   ; CHECK-NODEVIRT-IR: %call3 = tail call i32 %fptr22
   %call3 = tail cal

[llvm-branch-commits] [lld] d3f1f58 - [Docs] Add/update release notes for D71913 (LTO WPD changes)

[Teresa Johnson via llvm-branch-commits]

Author: Teresa Johnson
Date: 2020-09-15T08:51:56-07:00
New Revision: d3f1f588f902a968f102d6cdaf052674efc257aa

URL: 
https://github.com/llvm/llvm-project/commit/d3f1f588f902a968f102d6cdaf052674efc257aa
DIFF: 
https://github.com/llvm/llvm-project/commit/d3f1f588f902a968f102d6cdaf052674efc257aa.diff

LOG: [Docs] Add/update release notes for D71913 (LTO WPD changes)

This adds documentation for the options added / changed by D71913, which
enabled aggressive WPD under LTO. The lld release notes already
mentioned it, but I expanded the note.

Differential Revision: https://reviews.llvm.org/D86958

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
lld/docs/ReleaseNotes.rst
llvm/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ee257194d57f..d90b8f182ef9 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -306,7 +306,8 @@ Modified Compiler Flags
 - -mcpu is now supported for RISC-V, and recognises the generic-rv32,
   rocket-rv32, sifive-e31, generic-rv64, rocket-rv64, and sifive-u54 target
   CPUs.
-
+- ``-fwhole-program-vtables`` (along with ``-flto*``) now prepares all classes 
for possible whole program visibility if specified during the LTO link.
+  (`D71913 `_)
 
 New Pragmas in Clang
 

diff  --git a/lld/docs/ReleaseNotes.rst b/lld/docs/ReleaseNotes.rst
index 466a7f707354..880f933e51be 100644
--- a/lld/docs/ReleaseNotes.rst
+++ b/lld/docs/ReleaseNotes.rst
@@ -26,7 +26,7 @@ ELF Improvements
 
 * ``--lto-emit-asm`` is added to emit assembly output for debugging purposes.
   (`D77231 `_)
-* ``--lto-whole-program-visibility`` is added to support LTO whole-program 
devirtualization.
+* ``--lto-whole-program-visibility`` is added to specify that classes have 
hidden LTO visibility in LTO and ThinLTO links of source files compiled with 
``-fwhole-program-vtables``. See `LTOVisibility 
`_ for details.
   (`D71913 `_)
 * ``--print-archive-stats=`` is added to print the number of members and the 
number of fetched members for each archive.
   The feature is similar to GNU gold's ``--print-symbol-counts=``.

diff  --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst
index 0d5e0137bbc4..e87bf3d146f5 100644
--- a/llvm/docs/ReleaseNotes.rst
+++ b/llvm/docs/ReleaseNotes.rst
@@ -333,6 +333,12 @@ Changes to the Debug Info
   passed to the callee. The feature improves the debugging user experience when
   debugging optimized code.
 
+Changes to the Gold Plugin
+--
+
+* ``--plugin-opt=whole-program-visibility`` is added to specify that classes 
have hidden LTO visibility in LTO and ThinLTO links of source files compiled 
with ``-fwhole-program-vtables``. See `LTOVisibility 
`_ for details.
+  (`D71913 `_)
+
 Changes to the LLVM tools
 -
 



___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] 07f234b - [lld] Add --no-lto-whole-program-visibility

[Teresa Johnson via llvm-branch-commits]

Author: Teresa Johnson
Date: 2020-11-24T16:46:08-08:00
New Revision: 07f234be1ccbce131704f580aa3f117083a887f7

URL: 
https://github.com/llvm/llvm-project/commit/07f234be1ccbce131704f580aa3f117083a887f7
DIFF: 
https://github.com/llvm/llvm-project/commit/07f234be1ccbce131704f580aa3f117083a887f7.diff

LOG: [lld] Add --no-lto-whole-program-visibility

Enables overriding earlier --lto-whole-program-visibility.

Variant of D91583 while discussing alternate ways to identify and
handle the --export-dynamic case.

Differential Revision: https://reviews.llvm.org/D92060

Added: 


Modified: 
lld/ELF/Driver.cpp
lld/ELF/Options.td
lld/test/ELF/lto/devirt_vcall_vis_public.ll

Removed: 




diff  --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 783b85e7e27a6..c3f3d88b5d2d4 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -990,7 +990,8 @@ static void readConfigs(opt::InputArgList &args) {
   config->ltoNewPassManager = args.hasArg(OPT_lto_new_pass_manager);
   config->ltoNewPmPasses = args.getLastArgValue(OPT_lto_newpm_passes);
   config->ltoWholeProgramVisibility =
-  args.hasArg(OPT_lto_whole_program_visibility);
+  args.hasFlag(OPT_lto_whole_program_visibility,
+   OPT_no_lto_whole_program_visibility, false);
   config->ltoo = args::getInteger(args, OPT_lto_O, 2);
   config->ltoObjPath = args.getLastArgValue(OPT_lto_obj_path_eq);
   config->ltoPartitions = args::getInteger(args, OPT_lto_partitions, 1);

diff  --git a/lld/ELF/Options.td b/lld/ELF/Options.td
index 5171c08a8291f..db1c5d9698423 100644
--- a/lld/ELF/Options.td
+++ b/lld/ELF/Options.td
@@ -539,8 +539,9 @@ def lto_cs_profile_file: JJ<"lto-cs-profile-file=">,
 def lto_obj_path_eq: JJ<"lto-obj-path=">;
 def lto_sample_profile: JJ<"lto-sample-profile=">,
   HelpText<"Sample profile file path">;
-def lto_whole_program_visibility: FF<"lto-whole-program-visibility">,
-  HelpText<"Asserts that the LTO link has whole program visibility">;
+defm lto_whole_program_visibility: BB<"lto-whole-program-visibility",
+  "Asserts that the LTO link has whole program visibility",
+  "Asserts that the LTO link does not have whole program visibility">;
 def disable_verify: F<"disable-verify">;
 defm mllvm: Eq<"mllvm", "Additional arguments to forward to LLVM's option 
processing">;
 def opt_remarks_filename: Separate<["--"], "opt-remarks-filename">,

diff  --git a/lld/test/ELF/lto/devirt_vcall_vis_public.ll 
b/lld/test/ELF/lto/devirt_vcall_vis_public.ll
index 0d344ec9483df..46e23537d4fa5 100644
--- a/lld/test/ELF/lto/devirt_vcall_vis_public.ll
+++ b/lld/test/ELF/lto/devirt_vcall_vis_public.ll
@@ -1,6 +1,9 @@
 ; REQUIRES: x86
 ; Test that --lto-whole-program-visibility enables devirtualization.
 
+; Note that the --export-dynamic used below is simply to ensure symbols are
+; retained during linking.
+
 ; Index based WPD
 ; Generate unsplit module with summary for ThinLTO index-based WPD.
 ; RUN: opt --thinlto-bc -o %t2.o %s
@@ -31,6 +34,10 @@
 ; RUN: ld.lld %t2.o -o %t3 -save-temps \
 ; RUN:  -mllvm -pass-remarks=. --export-dynamic 2>&1 | FileCheck %s 
--implicit-check-not single-impl --allow-empty
 ; RUN: llvm-dis %t2.o.4.opt.bc -o - | FileCheck %s 
--check-prefix=CHECK-NODEVIRT-IR
+; Ensure --no-lto-whole-program-visibility overrides explicit 
--lto-whole-program-visibility.
+; RUN: ld.lld %t2.o -o %t3 -save-temps --lto-whole-program-visibility 
--no-lto-whole-program-visibility \
+; RUN:  -mllvm -pass-remarks=. --export-dynamic 2>&1 | FileCheck %s 
--implicit-check-not single-impl --allow-empty
+; RUN: llvm-dis %t2.o.4.opt.bc -o - | FileCheck %s 
--check-prefix=CHECK-NODEVIRT-IR
 
 ; Hybrid WPD
 ; RUN: ld.lld %t.o -o %t3 -save-temps \



___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] 77b5097 - [ICP] Don't promote when target not defined in module

[Teresa Johnson via llvm-branch-commits]

Author: Teresa Johnson
Date: 2020-12-08T07:45:36-08:00
New Revision: 77b509710ce7e6aec9ab460bc22a34f44fa94ab3

URL: 
https://github.com/llvm/llvm-project/commit/77b509710ce7e6aec9ab460bc22a34f44fa94ab3
DIFF: 
https://github.com/llvm/llvm-project/commit/77b509710ce7e6aec9ab460bc22a34f44fa94ab3.diff

LOG: [ICP] Don't promote when target not defined in module

This guards against cases where the symbol was dead code eliminated in
the binary by ThinLTO, and we have a sample profile collected for one
binary but used to optimize another.

Most of the benefit from ICP comes from inlining the target, which we
can't do with only a declaration anyway. If this is in the pre-ThinLTO
link step (e.g. for instrumentation based PGO), we will attempt the
promotion again in the ThinLTO backend after importing anyway, and we
don't need the early promotion to facilitate that.

Differential Revision: https://reviews.llvm.org/D92804

Added: 


Modified: 
llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp
llvm/test/LTO/Resolution/X86/load-sample-prof-icp.ll
llvm/test/Transforms/PGOProfile/icp_covariant_call_return.ll
llvm/test/Transforms/PGOProfile/icp_covariant_invoke_return.ll
llvm/test/Transforms/PGOProfile/indirect_call_promotion_musttail.ll

Removed: 




diff  --git a/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp 
b/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp
index bcd4e2e8e33c..5b9557a9b328 100644
--- a/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp
+++ b/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp
@@ -263,8 +263,15 @@ ICallPromotionFunc::getPromotionCandidatesForCallSite(
   break;
 }
 
+// Don't promote if the symbol is not defined in the module. This avoids
+// creating a reference to a symbol that doesn't exist in the module
+// This can happen when we compile with a sample profile collected from
+// one binary but used for another, which may have profiled targets that
+// aren't used in the new binary. We might have a declaration initially in
+// the case where the symbol is globally dead in the binary and removed by
+// ThinLTO.
 Function *TargetFunction = Symtab->getFunction(Target);
-if (TargetFunction == nullptr) {
+if (TargetFunction == nullptr || TargetFunction->isDeclaration()) {
   LLVM_DEBUG(dbgs() << " Not promote: Cannot find the target\n");
   ORE.emit([&]() {
 return OptimizationRemarkMissed(DEBUG_TYPE, "UnableToFindTarget", &CB)

diff  --git a/llvm/test/LTO/Resolution/X86/load-sample-prof-icp.ll 
b/llvm/test/LTO/Resolution/X86/load-sample-prof-icp.ll
index 8ebb0a134ffd..d4af2c4cc447 100644
--- a/llvm/test/LTO/Resolution/X86/load-sample-prof-icp.ll
+++ b/llvm/test/LTO/Resolution/X86/load-sample-prof-icp.ll
@@ -3,11 +3,11 @@
 ;
 ; RUN: opt -module-summary < %s -o %t.bc
 ; RUN: llvm-lto2 run -o %t.out %t.bc -save-temps \
-; RUN:   -r %t.bc,test,px -r %t.bc,bar,x \
+; RUN:   -r %t.bc,test,px -r %t.bc,bar,px -r %t.bc,externfunc,x \
 ; RUN:   -lto-sample-profile-file=%S/Inputs/load-sample-prof-icp.prof
 ; RUN: llvm-dis %t.out.1.4.opt.bc -o - | FileCheck %s
 ; RUN: llvm-lto2 run -o %t.out %t.bc -save-temps \
-; RUN:   -r %t.bc,test,px -r %t.bc,bar,x -use-new-pm \
+; RUN:   -r %t.bc,test,px -r %t.bc,bar,px -r %t.bc,externfunc,x -use-new-pm \
 ; RUN:   -lto-sample-profile-file=%S/Inputs/load-sample-prof-icp.prof
 ; RUN: llvm-dis %t.out.1.4.opt.bc -o - | FileCheck %s
 
@@ -26,9 +26,14 @@ define void @test(void ()*) #0 !dbg !7 {
   ret void
 }
 
-declare void @bar() local_unnamed_addr
+declare void @externfunc()
 
-attributes #0 = {"use-sample-profile"}
+define void @bar() #0 {
+  call void @externfunc()
+  ret void
+}
+
+attributes #0 = {"use-sample-profile" noinline}
 
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!3, !4, !5}

diff  --git a/llvm/test/Transforms/PGOProfile/icp_covariant_call_return.ll 
b/llvm/test/Transforms/PGOProfile/icp_covariant_call_return.ll
index c2101a98d0d1..a4f10c64170c 100644
--- a/llvm/test/Transforms/PGOProfile/icp_covariant_call_return.ll
+++ b/llvm/test/Transforms/PGOProfile/icp_covariant_call_return.ll
@@ -10,7 +10,9 @@ target triple = "x86_64-unknown-linux-gnu"
 
 declare noalias i8* @_Znwm(i64)
 declare void @_ZN1DC2Ev(%struct.D*);
-declare %struct.Derived* @_ZN1D4funcEv(%struct.D*);
+define %struct.Derived* @_ZN1D4funcEv(%struct.D*) {
+  ret %struct.Derived* null
+}
 
 define %struct.Base* @bar() {
 entry:

diff  --git a/llvm/test/Transforms/PGOProfile/icp_covariant_invoke_return.ll 
b/llvm/test/Transforms/PGOProfile/icp_covariant_invoke_return.ll
index d0f27ac73e78..c36b2e90ec7a 100644
--- a/llvm/test/Transforms/PGOProfile/icp_covariant_invoke_return.ll
+++ b/llvm/test/Transforms/PGOProfile/icp_covariant_invoke_return.ll
@@ -10,7 +10,9 @@ target triple = "x86_64-unknown-linux-gnu"
 @_ZTIi = external constant i8*
 declare i8* @_Znw

[llvm-branch-commits] [compiler-rt] d7e71b5 - [compiler-rt santizer] Use clock_gettime instead of timespec_get

[Teresa Johnson via llvm-branch-commits]

Author: Jeroen Dobbelaere
Date: 2020-12-08T10:10:17-08:00
New Revision: d7e71b5db8427309ad12249633ea86cccdc5a701

URL: 
https://github.com/llvm/llvm-project/commit/d7e71b5db8427309ad12249633ea86cccdc5a701
DIFF: 
https://github.com/llvm/llvm-project/commit/d7e71b5db8427309ad12249633ea86cccdc5a701.diff

LOG: [compiler-rt santizer] Use clock_gettime instead of timespec_get

On RH66, timespec_get is not available. Use clock_gettime instead.

This problem was introduced with D87120

Reviewed By: tejohnson

Differential Revision: https://reviews.llvm.org/D91687

Added: 


Modified: 
compiler-rt/lib/memprof/memprof_allocator.cpp
compiler-rt/lib/memprof/memprof_rtl.cpp

Removed: 




diff  --git a/compiler-rt/lib/memprof/memprof_allocator.cpp 
b/compiler-rt/lib/memprof/memprof_allocator.cpp
index 0f9263e66fe0..259c7c144ab7 100644
--- a/compiler-rt/lib/memprof/memprof_allocator.cpp
+++ b/compiler-rt/lib/memprof/memprof_allocator.cpp
@@ -52,7 +52,7 @@ static int GetTimestamp(void) {
 return 0;
   }
   timespec ts;
-  timespec_get(&ts, TIME_UTC);
+  clock_gettime(CLOCK_REALTIME, &ts);
   return (ts.tv_sec - memprof_init_timestamp_s) * 1000 + ts.tv_nsec / 100;
 }
 

diff  --git a/compiler-rt/lib/memprof/memprof_rtl.cpp 
b/compiler-rt/lib/memprof/memprof_rtl.cpp
index 8e1335aea875..d6d606f666ee 100644
--- a/compiler-rt/lib/memprof/memprof_rtl.cpp
+++ b/compiler-rt/lib/memprof/memprof_rtl.cpp
@@ -234,7 +234,7 @@ void MemprofInitTime() {
   if (LIKELY(memprof_timestamp_inited))
 return;
   timespec ts;
-  timespec_get(&ts, TIME_UTC);
+  clock_gettime(CLOCK_REALTIME, &ts);
   memprof_init_timestamp_s = ts.tv_sec;
   memprof_timestamp_inited = 1;
 }



___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [Metadata] Preserve MD_prof when merging instructions when one is missing. (PR #132433)

[Teresa Johnson via llvm-branch-commits]

@@ -0,0 +1,62 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py 
UTC_ARGS: --check-globals --version 2
+; RUN: opt < %s -passes='simplifycfg' 
-simplifycfg-require-and-preserve-domtree=1 -S | FileCheck %s 
--check-prefix=HOIST
+
+; Test case based on C++ code with manualy annotated !prof metadata.
+; This is to test that when calls to 'func1' from 'if.then' block
+; and 'if.else' block are hoisted, the branch_weights are merged and
+; attached to merged call rather than dropped.
+;
+; int func1(int a, int b) ;
+; int func2(int a, int b) ;
+
+; int func(int a, int b, bool c) {
+;int sum= 0;
+;if(c) {
+;sum += func1(a, b);
+;} else {
+;sum += func1(a, b);
+;sum -= func2(a, b);
+;}
+;return sum;
+; }
+define i32 @_Z4funciib(i32 %a, i32 %b, i1 %c) {
+; HOIST-LABEL: define i32 @_Z4funciib
+; HOIST-SAME: (i32 [[A:%.*]], i32 [[B:%.*]], i1 [[C:%.*]]) {
+; HOIST-NEXT:  entry:
+; HOIST-NEXT:[[CALL:%.*]] = tail call i32 @_Z5func1ii(i32 [[A]], i32 
[[B]]), !prof [[PROF0:![0-9]+]]
+; HOIST-NEXT:br i1 [[C]], label [[IF_END:%.*]], label [[IF_ELSE:%.*]]
+; HOIST:   if.else:
+; HOIST-NEXT:[[CALL3:%.*]] = tail call i32 @_Z5func2ii(i32 [[A]], i32 
[[B]])
+; HOIST-NEXT:[[SUB:%.*]] = sub i32 [[CALL]], [[CALL3]]
+; HOIST-NEXT:br label [[IF_END]]
+; HOIST:   if.end:
+; HOIST-NEXT:[[SUM_0:%.*]] = phi i32 [ [[SUB]], [[IF_ELSE]] ], [ [[CALL]], 
[[ENTRY:%.*]] ]
+; HOIST-NEXT:ret i32 [[SUM_0]]
+;
+entry:
+  br i1 %c, label %if.then, label %if.else
+
+if.then:  ; preds = %entry
+  %call = tail call i32 @_Z5func1ii(i32 %a, i32 %b), !prof !0
+  br label %if.end
+
+if.else:  ; preds = %entry
+  %call1 = tail call i32 @_Z5func1ii(i32 %a, i32 %b)
+  %call3 = tail call i32 @_Z5func2ii(i32 %a, i32 %b)
+  %sub = sub i32 %call1, %call3
+  br label %if.end
+
+if.end:   ; preds = %if.else, %if.then
+  %sum.0 = phi i32 [ %call, %if.then ], [ %sub, %if.else ]
+  ret i32 %sum.0
+}
+
+declare i32 @_Z5func1ii(i32, i32)
+
+declare i32 @_Z5func2ii(i32, i32)
+
+!0 = !{!"branch_weights", i32 10}
+!1 = !{!"branch_weights", i32 90}

teresajohnson wrote:

Unused in this test?

https://github.com/llvm/llvm-project/pull/132433
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [Metadata] Preserve MD_prof when merging instructions when one is missing. (PR #132433)

[Teresa Johnson via llvm-branch-commits]

@@ -3436,6 +3433,16 @@ static void combineMetadata(Instruction *K, const 
Instruction *J,
 K->setMetadata(LLVMContext::MD_callsite,
MDNode::getMergedCallsiteMetadata(KCallSite, JCallSite));
   }
+
+  // Merge prof metadata.
+  // Handle separately to support cases where only one instruction has the
+  // metadata.
+  auto JProf = J->getMetadata(LLVMContext::MD_prof);
+  auto KProf = K->getMetadata(LLVMContext::MD_prof);
+  if (!AAOnly && (JProf || KProf)) {

teresajohnson wrote:

The old handling was guarded on DoesKMove - what is the implication of removing 
that?

https://github.com/llvm/llvm-project/pull/132433
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [Metadata] Preserve MD_prof when merging instructions when one is missing. (PR #132433)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson approved this pull request.


https://github.com/llvm/llvm-project/pull/132433
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] llvm-reduce: Preserve uselistorder when writing thinlto bitcode (PR #133369)

[Teresa Johnson via llvm-branch-commits]

@@ -769,7 +769,11 @@ void ReducerWorkItem::readBitcode(MemoryBufferRef Data, 
LLVMContext &Ctx,
 }
 
 void ReducerWorkItem::writeBitcode(raw_ostream &OutStream) const {
+  const bool ShouldPreserveUseListOrder = true;
+
   if (LTOInfo && LTOInfo->IsThinLTO && LTOInfo->EnableSplitLTOUnit) {
+// FIXME: This should not depend on the pass manager. There are hidden

teresajohnson wrote:

It is necessary because the pass may mutate the module by splitting into 
regular and ThinLTO sub modules. See my comment on your other PR about how to 
provoke this splitting.

https://github.com/llvm/llvm-project/pull/133369
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] llvm-reduce: Preserve uselistorder when writing thinlto bitcode (PR #133369)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson approved this pull request.

lgtm but please remove the FIXME for the reason noted below

https://github.com/llvm/llvm-project/pull/133369
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] llvm-reduce: Preserve uselistorder when writing thinlto bitcode (PR #133369)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson edited 
https://github.com/llvm/llvm-project/pull/133369
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [NFC][MemProf] Move Radix tree methods to their own header and cpp. (PR #140501)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson approved this pull request.


https://github.com/llvm/llvm-project/pull/140501
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [NFC][MemProf] Move IndexedMemProfData to its own header. (PR #140503)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson approved this pull request.


https://github.com/llvm/llvm-project/pull/140503
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [NFC][MemProf] Move getGUID out of IndexedMemProfRecord (PR #140502)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson approved this pull request.


https://github.com/llvm/llvm-project/pull/140502
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [NFC][MemProf] Add the LLVM license text and minor clean up. (PR #140504)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson approved this pull request.


https://github.com/llvm/llvm-project/pull/140504
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [NFC][MemProf] Add the LLVM license text and minor clean up. (PR #140504)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson edited 
https://github.com/llvm/llvm-project/pull/140504
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [NFC][MemProf] Move types shared between Analysis, ProfileData and ModuleSummary (Core) to a separate header (PR #140505)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson approved this pull request.

lgtm with one question

https://github.com/llvm/llvm-project/pull/140505
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [NFC][MemProf] Move types shared between Analysis, ProfileData and ModuleSummary (Core) to a separate header (PR #140505)

[Teresa Johnson via llvm-branch-commits]

@@ -46,6 +46,7 @@
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 #include 
 #include 
+#include 

teresajohnson wrote:

Why this change? Were we pulling it in from ModuleSummaryIndex.h via some other 
chain of includes?

https://github.com/llvm/llvm-project/pull/140505
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [NFC][MemProf] Move types shared between Analysis, ProfileData and ModuleSummary (Core) to a separate header (PR #140505)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson edited 
https://github.com/llvm/llvm-project/pull/140505
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [NFC][MemProf] Move IndexedMemProfData to its own header. (PR #140503)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson edited 
https://github.com/llvm/llvm-project/pull/140503
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [NFC][MemProf] Move getGUID out of IndexedMemProfRecord (PR #140502)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson edited 
https://github.com/llvm/llvm-project/pull/140502
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [NFC][MemProf] Move Radix tree methods to their own header and cpp. (PR #140501)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson edited 
https://github.com/llvm/llvm-project/pull/140501
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] Add SimplifyTypeTests pass. (PR #141327)

[Teresa Johnson via llvm-branch-commits]

@@ -2478,3 +2479,76 @@ PreservedAnalyses LowerTypeTestsPass::run(Module &M,
 return PreservedAnalyses::all();
   return PreservedAnalyses::none();
 }
+
+PreservedAnalyses SimplifyTypeTestsPass::run(Module &M,
+ ModuleAnalysisManager &AM) {
+  bool Changed = false;
+  // Figure out whether inlining has exposed a constant address to a lowered
+  // type test, and remove the test if so and the address is known to pass the
+  // test. Unfortunately this pass ends up needing to reverse engineer what
+  // LowerTypeTests did; this is currently inherent to the design of ThinLTO
+  // importing where LowerTypeTests needs to run at the start.
+  for (auto &GV : M.globals()) {
+if (!GV.getName().starts_with("__typeid_") ||
+!GV.getName().ends_with("_global_addr"))
+  continue;
+auto *MD = MDString::get(M.getContext(),
+ GV.getName().substr(9, GV.getName().size() - 21));
+auto MaySimplifyPtr = [&](Value *Ptr) {
+  if (auto *GV = dyn_cast(Ptr))
+if (auto *CFIGV = M.getNamedValue((GV->getName() + ".cfi").str()))
+  Ptr = CFIGV;
+  return isKnownTypeIdMember(MD, M.getDataLayout(), Ptr, 0);

teresajohnson wrote:

Are there cases where the GV will not have a ".cfi" extension? I notice the 
test has that extension.

https://github.com/llvm/llvm-project/pull/141327
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] Add SimplifyTypeTests pass. (PR #141327)

[Teresa Johnson via llvm-branch-commits]

@@ -2478,3 +2479,76 @@ PreservedAnalyses LowerTypeTestsPass::run(Module &M,
 return PreservedAnalyses::all();
   return PreservedAnalyses::none();
 }
+
+PreservedAnalyses SimplifyTypeTestsPass::run(Module &M,
+ ModuleAnalysisManager &AM) {
+  bool Changed = false;
+  // Figure out whether inlining has exposed a constant address to a lowered
+  // type test, and remove the test if so and the address is known to pass the
+  // test. Unfortunately this pass ends up needing to reverse engineer what
+  // LowerTypeTests did; this is currently inherent to the design of ThinLTO

teresajohnson wrote:

Can you add a more extensive comment with what this is looking for and why? I 
don't look at lower type test output often so I don't recall offhand what e.g. 
it would have looked like without inlining vs with.

https://github.com/llvm/llvm-project/pull/141327
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] Add SimplifyTypeTests pass. (PR #141327)

[Teresa Johnson via llvm-branch-commits]

@@ -0,0 +1,40 @@
+; RUN: opt -S %s -passes=simplify-type-tests | FileCheck %s

teresajohnson wrote:

Add a comment about what this is testing

https://github.com/llvm/llvm-project/pull/141327
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] Add SimplifyTypeTests pass. (PR #141327)

[Teresa Johnson via llvm-branch-commits]

@@ -2478,3 +2479,76 @@ PreservedAnalyses LowerTypeTestsPass::run(Module &M,
 return PreservedAnalyses::all();
   return PreservedAnalyses::none();
 }
+
+PreservedAnalyses SimplifyTypeTestsPass::run(Module &M,
+ ModuleAnalysisManager &AM) {
+  bool Changed = false;
+  // Figure out whether inlining has exposed a constant address to a lowered
+  // type test, and remove the test if so and the address is known to pass the
+  // test. Unfortunately this pass ends up needing to reverse engineer what
+  // LowerTypeTests did; this is currently inherent to the design of ThinLTO
+  // importing where LowerTypeTests needs to run at the start.
+  for (auto &GV : M.globals()) {
+if (!GV.getName().starts_with("__typeid_") ||
+!GV.getName().ends_with("_global_addr"))
+  continue;
+auto *MD = MDString::get(M.getContext(),

teresajohnson wrote:

Can you add a comment on this conversion? Figured it out by adding up the chars 
myself but it would be good to make it explicit.

https://github.com/llvm/llvm-project/pull/141327
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] llvm-lto2: Add print-guid subcommand. (PR #143992)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson commented:

This needs a caveat somewhere (either in printed usage message or in a comment) 
that this won't work for local linkage symbols (I suppose the user could give 
the "file:" prefix but that won't work if -funique-internal-linkage-names was 
specified etc). Can you also add a test?

https://github.com/llvm/llvm-project/pull/143992
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] llvm-lto2: Add print-guid subcommand. (PR #143992)

[Teresa Johnson via llvm-branch-commits]

teresajohnson wrote:

> > This needs a caveat somewhere (either in printed usage message or in a 
> > comment) that this won't work for local linkage symbols (I suppose the user 
> > could give the "file:" prefix but that won't work if 
> > -funique-internal-linkage-names was specified etc).
> 
> I'm not sure that is worth it. The intent is that users of these development 
> tools will refer to the source code. And if you read the source code you'll 
> see the function name `getGUIDAssumingExternalLinkage` which tells you what 
> you need to know.

At least a comment in the code would be good. A variety of people end up using 
these tools for tests, and I could see someone getting confused as to why the 
guid doesn't match what's e.g. in the ThinLTO index. For that understanding 
you'd have to read more than just what this source file is calling.

https://github.com/llvm/llvm-project/pull/143992
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] llvm-lto2: Add print-guid subcommand. (PR #143992)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson approved this pull request.

lgtm otherwise

https://github.com/llvm/llvm-project/pull/143992
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] Add SimplifyTypeTests pass. (PR #141327)

[Teresa Johnson via llvm-branch-commits]

https://github.com/teresajohnson approved this pull request.

lgtm but I think there is a code formatting error reported that should be fixed 
before merging.

https://github.com/llvm/llvm-project/pull/141327
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits