modimo updated this revision to Diff 374723.
modimo marked 3 inline comments as done.
modimo added a comment.
Address follow-ups
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D36850/new/
https://reviews.llvm.org/D36850
Files:
clang/test/CodeGen/thinlto-funcattr-prop.ll
llvm/include/llvm/Transforms/IPO/FunctionImport.h
llvm/lib/Transforms/IPO/FunctionAttrs.cpp
llvm/lib/Transforms/IPO/FunctionImport.cpp
llvm/test/ThinLTO/X86/funcattrs-prop-maythrow.ll
llvm/test/ThinLTO/X86/funcattrs-prop.ll
Index: llvm/test/ThinLTO/X86/funcattrs-prop.ll
===================================================================
--- llvm/test/ThinLTO/X86/funcattrs-prop.ll
+++ llvm/test/ThinLTO/X86/funcattrs-prop.ll
@@ -3,8 +3,8 @@
; RUN: opt -module-summary %t/b.ll -o %t/b.bc
; RUN: opt -module-summary %t/c.ll -o %t/c.bc
-;; ThinLTO Function attribute propagation uses the prevailing symbol to propagate attributes to its callers. Interposable (linkonce and weak) linkages are fair game given we know the prevailing copy
-;; will be used in the final binary.
+;; ThinLTO Function attribute propagation uses the prevailing symbol to propagate attributes to its callers.
+;; Interposable (linkonce and weak) linkages are fair game given we know the prevailing copy will be used in the final binary.
; RUN: llvm-lto2 run -disable-thinlto-funcattrs=0 %t/a.bc %t/b.bc %t/c.bc -o %t1 -save-temps \
; RUN: -r=%t/a.bc,call_extern,plx -r=%t/a.bc,call_linkonceodr,plx -r=%t/a.bc,call_weakodr,plx -r=%t/a.bc,call_linkonce,plx -r=%t/a.bc,call_weak,plx -r=%t/a.bc,call_linkonce_may_unwind,plx -r=%t/a.bc,call_weak_may_unwind,plx \
; RUN: -r=%t/a.bc,extern, -r=%t/a.bc,linkonceodr, -r=%t/a.bc,weakodr, -r=%t/a.bc,linkonce, -r=%t/a.bc,weak, -r=%t/a.bc,linkonce_may_unwind, -r=%t/a.bc,weak_may_unwind, \
Index: llvm/test/ThinLTO/X86/funcattrs-prop-maythrow.ll
===================================================================
--- llvm/test/ThinLTO/X86/funcattrs-prop-maythrow.ll
+++ llvm/test/ThinLTO/X86/funcattrs-prop-maythrow.ll
@@ -2,9 +2,9 @@
; RUN: split-file %s %t
; RUN: opt -thinlto-bc %t/main.ll -thin-link-bitcode-file=%t1.thinlink.bc -o %t1.bc
; RUN: opt -thinlto-bc %t/callees.ll -thin-link-bitcode-file=%t2.thinlink.bc -o %t2.bc
-; RUN: llvm-lto2 run -disable-thinlto-funcattrs=0 %t1.bc %t2.bc -o %t.o -r %t1.bc,caller,px -r %t1.bc,caller1,px -r %t1.bc,caller2,px \
-; RUN: -r %t1.bc,cleanupret,l -r %t1.bc,catchret,l -r %t1.bc,resume,l \
-; RUN: -r %t2.bc,cleanupret,px -r %t2.bc,catchret,px -r %t2.bc,resume,px -r %t2.bc,nonThrowing,px -r %t2.bc,__gxx_personality_v0,px -save-temps
+; RUN: llvm-lto2 run -disable-thinlto-funcattrs=0 %t1.bc %t2.bc -o %t.o -r %t1.bc,caller,px -r %t1.bc,caller1,px -r %t1.bc,caller2,px -r %t1.bc,caller_nounwind,px \
+; RUN: -r %t1.bc,cleanupret,l -r %t1.bc,catchret,l -r %t1.bc,resume,l -r %t1.bc,cleanupret_nounwind,l \
+; RUN: -r %t2.bc,cleanupret,px -r %t2.bc,catchret,px -r %t2.bc,resume,px -r %t2.bc,cleanupret_nounwind,px -r %t2.bc,nonThrowing,px -r %t2.bc,__gxx_personality_v0,px -save-temps
; RUN: llvm-dis -o - %t2.bc | FileCheck %s --check-prefix=SUMMARY
; RUN: llvm-dis -o - %t.o.1.3.import.bc | FileCheck %s
@@ -16,26 +16,37 @@
declare void @catchret()
declare void @resume()
+; Functions can have mayThrow instructions but also be marked noUnwind
+; if they have terminate semantics (e.g. noexcept). In such cases
+; propagation trusts the original noUnwind value in the function summary
+declare void @cleanupret_nounwind()
-; CHECK: define void @caller() [[ATTR:#[0-9]+]]
+; CHECK: define void @caller() [[ATTR_MAYTHROW:#[0-9]+]]
define void @caller() {
call void @cleanupret()
ret void
}
-; CHECK: define void @caller1() [[ATTR:#[0-9]+]]
+; CHECK: define void @caller1() [[ATTR_MAYTHROW:#[0-9]+]]
define void @caller1() {
call void @catchret()
ret void
}
-; CHECK: define void @caller2() [[ATTR:#[0-9]+]]
+; CHECK: define void @caller2() [[ATTR_MAYTHROW:#[0-9]+]]
define void @caller2() {
call void @resume()
ret void
}
-; CHECK-DAG: attributes [[ATTR]] = { norecurse }
+; CHECK: define void @caller_nounwind() [[ATTR_NOUNWIND:#[0-9]+]]
+define void @caller_nounwind() {
+ call void @cleanupret_nounwind()
+ ret void
+}
+
+; CHECK-DAG: attributes [[ATTR_NOUNWIND]] = { norecurse nounwind }
+; CHECK-DAG: attributes [[ATTR_MAYTHROW]] = { norecurse }
; SUMMARY-DAG: = gv: (name: "cleanupret", summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 4, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 0, mayThrow: 1, hasUnknownCall: 0), calls: ((callee: ^{{.*}})), refs: (^{{.*}}))))
; SUMMARY-DAG: = gv: (name: "resume", summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 4, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 0, mayThrow: 1, hasUnknownCall: 0), calls: ((callee: ^{{.*}})), refs: (^{{.*}}))))
@@ -89,3 +100,16 @@
cleanup
resume { i8*, i32 } %exn
}
+
+define void @cleanupret_nounwind() #0 personality i32 (...)* @__gxx_personality_v0 {
+entry:
+ invoke void @nonThrowing()
+ to label %exit unwind label %pad
+pad:
+ %cp = cleanuppad within none [i7 4]
+ cleanupret from %cp unwind to caller
+exit:
+ ret void
+}
+
+attributes #0 = { nounwind }
\ No newline at end of file
Index: llvm/lib/Transforms/IPO/FunctionImport.cpp
===================================================================
--- llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -1044,11 +1044,30 @@
void llvm::thinLTOFinalizeInModule(Module &TheModule,
const GVSummaryMapTy &DefinedGlobals,
bool PropagateAttrs) {
- auto updateLinkage = [&](GlobalValue &GV) {
+ auto FinalizeInModule = [&](GlobalValue &GV, bool Propagate = false) {
// See if the global summary analysis computed a new resolved linkage.
const auto &GS = DefinedGlobals.find(GV.getGUID());
if (GS == DefinedGlobals.end())
return;
+
+ if (Propagate)
+ if (FunctionSummary *FS = dyn_cast<FunctionSummary>(GS->second)) {
+ if (Function *F = dyn_cast<Function>(&GV)) {
+ // TODO: propagate ReadNone and ReadOnly.
+ if (FS->fflags().ReadNone && !F->doesNotAccessMemory())
+ F->setDoesNotAccessMemory();
+
+ if (FS->fflags().ReadOnly && !F->onlyReadsMemory())
+ F->setOnlyReadsMemory();
+
+ if (FS->fflags().NoRecurse && !F->doesNotRecurse())
+ F->setDoesNotRecurse();
+
+ if (FS->fflags().NoUnwind && !F->doesNotThrow())
+ F->setDoesNotThrow();
+ }
+ }
+
auto NewLinkage = GS->second->linkage();
if (GlobalValue::isLocalLinkage(GV.getLinkage()) ||
// Don't internalize anything here, because the code below
@@ -1105,37 +1124,13 @@
GO->setComdat(nullptr);
};
- auto propagateFunctionAttrs = [&](Function &F) {
- const auto &GV = DefinedGlobals.find(F.getGUID());
- if (GV == DefinedGlobals.end())
- return;
-
- if (FunctionSummary *FS = dyn_cast<FunctionSummary>(GV->second)) {
- // TODO: propagate ReadNone and ReadOnly.
- if (FS->fflags().ReadNone && !F.doesNotAccessMemory())
- F.setDoesNotAccessMemory();
-
- if (FS->fflags().ReadOnly && !F.onlyReadsMemory())
- F.setOnlyReadsMemory();
-
- if (FS->fflags().NoRecurse && !F.doesNotRecurse())
- F.setDoesNotRecurse();
-
- if (FS->fflags().NoUnwind && !F.doesNotThrow())
- F.setDoesNotThrow();
- }
- };
-
// Process functions and global now
- for (auto &GV : TheModule) {
- updateLinkage(GV);
- if (PropagateAttrs)
- propagateFunctionAttrs(GV);
- }
+ for (auto &GV : TheModule)
+ FinalizeInModule(GV, PropagateAttrs);
for (auto &GV : TheModule.globals())
- updateLinkage(GV);
+ FinalizeInModule(GV);
for (auto &GV : TheModule.aliases())
- updateLinkage(GV);
+ FinalizeInModule(GV);
}
/// Run internalization on \p TheModule based on symmary analysis.
Index: llvm/lib/Transforms/IPO/FunctionAttrs.cpp
===================================================================
--- llvm/lib/Transforms/IPO/FunctionAttrs.cpp
+++ llvm/lib/Transforms/IPO/FunctionAttrs.cpp
@@ -476,8 +476,7 @@
if (!CalleeSummary->fflags().NoRecurse)
InferredFlags.NoRecurse = false;
- if (!CalleeSummary->fflags().NoUnwind ||
- CalleeSummary->fflags().MayThrow)
+ if (!CalleeSummary->fflags().NoUnwind)
InferredFlags.NoUnwind = false;
if (!InferredFlags.NoUnwind && !InferredFlags.NoRecurse)
Index: llvm/include/llvm/Transforms/IPO/FunctionImport.h
===================================================================
--- llvm/include/llvm/Transforms/IPO/FunctionImport.h
+++ llvm/include/llvm/Transforms/IPO/FunctionImport.h
@@ -218,10 +218,11 @@
/// summary-based analysis:
/// 1. Resolve prevailing symbol linkages and constrain visibility (CanAutoHide
/// and consider visibility from other definitions for ELF) in \p TheModule
-/// 2. (optional) Apply propagated function attributes to \p TheModule
+/// 2. (optional) Apply propagated function attributes to \p TheModule if
+/// PropagateAttrs is true
void thinLTOFinalizeInModule(Module &TheModule,
const GVSummaryMapTy &DefinedGlobals,
- bool PropagateAttrs = false);
+ bool PropagateAttrs);
/// Internalize \p TheModule based on the information recorded in the summaries
/// during global summary-based analysis.
Index: clang/test/CodeGen/thinlto-funcattr-prop.ll
===================================================================
--- clang/test/CodeGen/thinlto-funcattr-prop.ll
+++ clang/test/CodeGen/thinlto-funcattr-prop.ll
@@ -11,10 +11,13 @@
; RUN: -r=%t/a.bc,extern, \
; RUN: -r=%t/b.bc,extern,p
-; RUN: llvm-dis %t1.index.bc -o - | FileCheck %s
+; RUN: llvm-dis %t1.o.index.bc -o - | FileCheck %s --check-prefix=CHECK-INDEX
+; RUN: llvm-dis %t1.o.1.1.promote.bc -o - | FileCheck %s --check-prefix=CHECK-IR
-; CHECK: ^2 = gv: (guid: 13959900437860518209, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 1, dsoLocal: 1, canAutoHide: 0), insts: 2, calls: ((callee: ^3)))))
-; CHECK: ^3 = gv: (guid: 14959766916849974397, summaries: (function: (module: ^1, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 1, dsoLocal: 0, canAutoHide: 0), insts: 1, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 1, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 1, mayThrow: 0, hasUnknownCall: 0))))
+;; Summary for call_extern. Note that llvm-lto2 writes out the index before
+; CHECK-INDEX: ^2 = gv: (guid: 13959900437860518209, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 1, dsoLocal: 1, canAutoHide: 0), insts: 2, calls: ((callee: ^3)))))
+;; Summary for extern
+; CHECK-INDEX: ^3 = gv: (guid: 14959766916849974397, summaries: (function: (module: ^1, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 1, dsoLocal: 0, canAutoHide: 0), insts: 1, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 1, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 1, mayThrow: 0, hasUnknownCall: 0))))
;--- a.ll
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
@@ -22,6 +25,8 @@
declare void @extern()
+; CHECK-IR: Function Attrs: norecurse nounwind
+; CHECK-IR-NEXT: define dso_local void @call_extern()
define void @call_extern() {
call void @extern()
ret void
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
