Skip to content

Commit 61a85da

Browse files
preamespreames
committed
[InferAttributes] Materialize all infered attributes for declaration
We have some cases today where attributes can be inferred from another on access, but the result is not explicitly materialized in IR. This change is a step towards changing that. Why? Two main reasons: * Human clarity. It's really confusing trying to figure out why a transform is triggering when the IR doesn't appear to have the required attributes. * This avoids the need to special case declarations in e.g. functionattrs. Since we can assume the attribute is present, we can work directly from attributes (and only attributes) without also needing to query accessors on Function to avoid missing cases due to unannotated (but infered on use) declarations. (This piece will appear must easier to follow once D100226 also lands.) Differential Revision: https://reviews.llvm.org/D100400
1 parent 6150001 commit 61a85da

File tree

4 files changed

+56
-19
lines changed

4 files changed

+56
-19
lines changed

‎llvm/lib/Transforms/IPO/InferFunctionAttrs.cpp

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,52 @@ using namespace llvm;
1919

2020
#define DEBUG_TYPE "inferattrs"
2121

22+
/// If we can infer one attribute from another on the declaration of a
23+
/// function, explicitly materialize the maximal set for readability in the IR.
24+
/// Doing this also allows our CGSCC inference to avoid needing to duplicate
25+
/// this logic on all calls to declarations (as declarations aren't explicitly
26+
/// visited by CGSCC passes in the new pass manager.)
27+
static bool inferAttributesFromOthers(Function &F) {
28+
// Note: We explicitly check for attributes rather than using cover functions
29+
// because some of the cover functions include the logic being implemented.
30+
31+
bool Changed = false;
32+
// readnone + not convergent implies nosync
33+
if (!F.hasFnAttribute(Attribute::NoSync) &&
34+
F.doesNotAccessMemory() && !F.isConvergent()) {
35+
F.setNoSync();
36+
Changed = true;
37+
}
38+
39+
// readonly implies nofree
40+
if (!F.hasFnAttribute(Attribute::NoFree) && F.onlyReadsMemory()) {
41+
F.setDoesNotFreeMemory();
42+
Changed = true;
43+
}
44+
45+
// willreturn implies mustprogress
46+
if (!F.hasFnAttribute(Attribute::MustProgress) && F.willReturn()) {
47+
F.setMustProgress();
48+
Changed = true;
49+
}
50+
51+
// TODO: There are a bunch of cases of restrictive memory effects we
52+
// can infer by inspecting arguments of argmemonly-ish functions.
53+
54+
return Changed;
55+
}
56+
2257
static bool inferAllPrototypeAttributes(
2358
Module &M, function_ref<TargetLibraryInfo &(Function &)> GetTLI) {
2459
bool Changed = false;
2560

2661
for (Function &F : M.functions())
2762
// We only infer things using the prototype and the name; we don't need
2863
// definitions.
29-
if (F.isDeclaration() && !F.hasOptNone())
64+
if (F.isDeclaration() && !F.hasOptNone()) {
3065
Changed |= inferLibFuncAttributes(F, GetTLI(F));
66+
Changed |= inferAttributesFromOthers(F);
67+
}
3168

3269
return Changed;
3370
}

‎llvm/test/Other/cgscc-devirt-iteration.ll

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,18 @@
44
; devirtualization here with GVN which forwards a store through a load and to
55
; an indirect call.
66
;
7-
; RUN: opt -aa-pipeline=basic-aa -passes='cgscc(function-attrs,function(gvn,instcombine))' -S < %s | FileCheck %s --check-prefix=CHECK --check-prefix=BEFORE
8-
; RUN: opt -aa-pipeline=basic-aa -passes='cgscc(devirt<1>(function-attrs,function(gvn,instcombine)))' -S < %s | FileCheck %s --check-prefix=CHECK --check-prefix=AFTER --check-prefix=AFTER1
9-
; RUN: opt -aa-pipeline=basic-aa -passes='cgscc(devirt<2>(function-attrs,function(gvn,instcombine)))' -S < %s | FileCheck %s --check-prefix=CHECK --check-prefix=AFTER --check-prefix=AFTER2
7+
; RUN: opt -aa-pipeline=basic-aa -passes='module(inferattrs),cgscc(function-attrs,function(gvn,instcombine))' -S < %s | FileCheck %s --check-prefix=CHECK --check-prefix=BEFORE
8+
; RUN: opt -aa-pipeline=basic-aa -passes='module(inferattrs),cgscc(devirt<1>(function-attrs,function(gvn,instcombine)))' -S < %s | FileCheck %s --check-prefix=CHECK --check-prefix=AFTER --check-prefix=AFTER1
9+
; RUN: opt -aa-pipeline=basic-aa -passes='module(inferattrs),cgscc(devirt<2>(function-attrs,function(gvn,instcombine)))' -S < %s | FileCheck %s --check-prefix=CHECK --check-prefix=AFTER --check-prefix=AFTER2
1010
;
11-
; RUN: not --crash opt -abort-on-max-devirt-iterations-reached -aa-pipeline=basic-aa -passes='cgscc(devirt<1>(function-attrs,function(gvn,instcombine)))' -S < %s
12-
; RUN: opt -abort-on-max-devirt-iterations-reached -aa-pipeline=basic-aa -passes='cgscc(devirt<2>(function-attrs,function(gvn,instcombine)))' -S < %s
11+
; RUN: not --crash opt -abort-on-max-devirt-iterations-reached -aa-pipeline=basic-aa -passes='module(inferattrs),cgscc(devirt<1>(function-attrs,function(gvn,instcombine)))' -S < %s
12+
; RUN: opt -abort-on-max-devirt-iterations-reached -aa-pipeline=basic-aa -passes='module(inferattrs),cgscc(devirt<2>(function-attrs,function(gvn,instcombine)))' -S < %s
1313
;
1414
; We also verify that the real O2 pipeline catches these cases.
1515
; RUN: opt -aa-pipeline=basic-aa -passes='default<O2>' -S < %s | FileCheck %s --check-prefix=CHECK --check-prefix=AFTER --check-prefix=AFTER2
1616

1717
declare void @readnone() readnone
18-
; CHECK: Function Attrs: readnone
18+
; CHECK: Function Attrs: nofree nosync readnone
1919
; CHECK-NEXT: declare void @readnone()
2020

2121
declare void @unknown()
@@ -51,7 +51,7 @@ entry:
5151
; devirtualize again, and then deduce readnone.
5252

5353
declare void @readnone_with_arg(void ()**) readnone
54-
; CHECK: Function Attrs: readnone
54+
; CHECK: Function Attrs: nofree nosync readnone
5555
; CHECK-LABEL: declare void @readnone_with_arg(void ()**)
5656

5757
define void @test2_a(void ()** %ignore) {

‎llvm/test/Transforms/InferFunctionAttrs/annotate.ll

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1016,18 +1016,18 @@ declare i64 @write(i32, i8*, i64)
10161016
declare void @memset_pattern16(i8*, i8*, i64)
10171017

10181018

1019-
; CHECK-DAG: attributes [[NOFREE_NOUNWIND_WILLRETURN]] = { nofree nounwind willreturn }
1019+
; CHECK-DAG: attributes [[NOFREE_NOUNWIND_WILLRETURN]] = { nofree nounwind willreturn mustprogress }
10201020
; CHECK-DAG: attributes [[NOFREE_NOUNWIND]] = { nofree nounwind }
1021-
; CHECK-DAG: attributes [[INACCESSIBLEMEMONLY_NOFREE_NOUNWIND_WILLRETURN]] = { inaccessiblememonly nofree nounwind willreturn }
1022-
; CHECK-DAG: attributes [[NOFREE_NOUNWIND_READONLY_WILLRETURN]] = { nofree nounwind readonly willreturn }
1023-
; CHECK-DAG: attributes [[ARGMEMONLY_NOFREE_NOUNWIND_WILLRETURN]] = { argmemonly nofree nounwind willreturn }
1021+
; CHECK-DAG: attributes [[INACCESSIBLEMEMONLY_NOFREE_NOUNWIND_WILLRETURN]] = { inaccessiblememonly nofree nounwind willreturn mustprogress }
1022+
; CHECK-DAG: attributes [[NOFREE_NOUNWIND_READONLY_WILLRETURN]] = { nofree nounwind readonly willreturn mustprogress }
1023+
; CHECK-DAG: attributes [[ARGMEMONLY_NOFREE_NOUNWIND_WILLRETURN]] = { argmemonly nofree nounwind willreturn mustprogress }
10241024
; CHECK-DAG: attributes [[NOFREE_NOUNWIND_READONLY]] = { nofree nounwind readonly }
1025-
; CHECK-DAG: attributes [[INACCESSIBLEMEMORARGMEMONLY_NOUNWIND_WILLRETURN]] = { inaccessiblemem_or_argmemonly nounwind willreturn }
1026-
; CHECK-DAG: attributes [[NOFREE_WILLRETURN]] = { nofree willreturn }
1027-
; CHECK-DAG: attributes [[ARGMEMONLY_NOFREE_NOUNWIND_READONLY_WILLRETURN]] = { argmemonly nofree nounwind readonly willreturn }
1025+
; CHECK-DAG: attributes [[INACCESSIBLEMEMORARGMEMONLY_NOUNWIND_WILLRETURN]] = { inaccessiblemem_or_argmemonly nounwind willreturn mustprogress }
1026+
; CHECK-DAG: attributes [[NOFREE_WILLRETURN]] = { nofree willreturn mustprogress }
1027+
; CHECK-DAG: attributes [[ARGMEMONLY_NOFREE_NOUNWIND_READONLY_WILLRETURN]] = { argmemonly nofree nounwind readonly willreturn mustprogress }
10281028
; CHECK-DAG: attributes [[NOFREE]] = { nofree }
1029-
; CHECK-DAG: attributes [[WILLRETURN]] = { willreturn }
1030-
; CHECK-DAG: attributes [[INACCESSIBLEMEMORARGONLY_NOFREE_NOUNWIND_WILLRETURN]] = { inaccessiblemem_or_argmemonly nofree nounwind willreturn }
1029+
; CHECK-DAG: attributes [[WILLRETURN]] = { willreturn mustprogress }
1030+
; CHECK-DAG: attributes [[INACCESSIBLEMEMORARGONLY_NOFREE_NOUNWIND_WILLRETURN]] = { inaccessiblemem_or_argmemonly nofree nounwind willreturn mustprogress }
10311031

10321032
; CHECK-DARWIN-DAG: attributes [[ARGMEMONLY_NOFREE]] = { argmemonly nofree }
1033-
; CHECK-NVPTX-DAG: attributes [[NOFREE_NOUNWIND_READNONE]] = { nofree nounwind readnone }
1033+
; CHECK-NVPTX-DAG: attributes [[NOFREE_NOUNWIND_READNONE]] = { nofree nosync nounwind readnone }

‎llvm/test/Transforms/LICM/strlen.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ loop:
1313
}
1414

1515
; CHECK: declare i64 @strlen(i8* nocapture) #0
16-
; CHECK: attributes #0 = { argmemonly nofree nounwind readonly willreturn }
16+
; CHECK: attributes #0 = { argmemonly nofree nounwind readonly willreturn mustprogress }
1717
declare i64 @strlen(i8*)
1818

1919

0 commit comments

Comments
 (0)