[llvm] d7be822 - [Attributor][FIX] Improve care when dealing with liveness

Johannes Doerfert via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 11 23:49:21 PST 2023


Author: Johannes Doerfert
Date: 2023-01-11T23:49:10-08:00
New Revision: d7be8227a84f95071911b60d32d751e8780dde12

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

LOG: [Attributor][FIX] Improve care when dealing with liveness

This patch adds two checks that have in experiments caused issues. One
was an oversight that allowed new AAs during cleanup to be optimistic.
The other treated functions as functions even if they were used as
values, e.g., in a cast instruction. In such cases we might have assumed
the value is dead if the function is not entered, which isn't true.

The new test functions don't expose a bug but I kept them around.

Added: 
    

Modified: 
    llvm/include/llvm/Transforms/IPO/Attributor.h
    llvm/lib/Transforms/IPO/Attributor.cpp
    llvm/test/Transforms/Attributor/IPConstantProp/openmp_parallel_for.ll
    llvm/test/Transforms/Attributor/align.ll
    llvm/test/Transforms/Attributor/value-simplify.ll

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Transforms/IPO/Attributor.h b/llvm/include/llvm/Transforms/IPO/Attributor.h
index 272d71076923a..41b555b8e5b7a 100644
--- a/llvm/include/llvm/Transforms/IPO/Attributor.h
+++ b/llvm/include/llvm/Transforms/IPO/Attributor.h
@@ -1596,7 +1596,8 @@ struct Attributor {
 
     // If this is queried in the manifest stage, we force the AA to indicate
     // pessimistic fixpoint immediately.
-    if (Phase == AttributorPhase::MANIFEST) {
+    if (Phase == AttributorPhase::MANIFEST ||
+        Phase == AttributorPhase::CLEANUP) {
       AA.getState().indicatePessimisticFixpoint();
       return AA;
     }

diff  --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index 16a1f82b7888f..50facdb56a290 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -1397,6 +1397,13 @@ bool Attributor::isAssumedDead(const IRPosition &IRP,
                                const AAIsDead *FnLivenessAA,
                                bool &UsedAssumedInformation,
                                bool CheckBBLivenessOnly, DepClassTy DepClass) {
+  // Don't check liveness for constants, e.g. functions, used as (floating)
+  // values since the context instruction and such is here meaningless.
+  if (IRP.getPositionKind() == IRPosition::IRP_FLOAT &&
+      isa<Constant>(IRP.getAssociatedValue())) {
+    return false;
+  }
+
   Instruction *CtxI = IRP.getCtxI();
   if (CtxI &&
       isAssumedDead(*CtxI, QueryingAA, FnLivenessAA, UsedAssumedInformation,

diff  --git a/llvm/test/Transforms/Attributor/IPConstantProp/openmp_parallel_for.ll b/llvm/test/Transforms/Attributor/IPConstantProp/openmp_parallel_for.ll
index bd7696682b63c..bcfe3658ca159 100644
--- a/llvm/test/Transforms/Attributor/IPConstantProp/openmp_parallel_for.ll
+++ b/llvm/test/Transforms/Attributor/IPConstantProp/openmp_parallel_for.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --check-attributes --check-globals
-; RUN: opt -aa-pipeline=basic-aa -passes=attributor -attributor-manifest-internal  -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=6 -S < %s | FileCheck %s --check-prefixes=CHECK,TUNIT
+; RUN: opt -aa-pipeline=basic-aa -passes=attributor -attributor-manifest-internal  -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=16 -S < %s | FileCheck %s --check-prefixes=CHECK,TUNIT
 ; RUN: opt -aa-pipeline=basic-aa -passes=attributor-cgscc -attributor-manifest-internal  -attributor-annotate-decl-cs -S < %s | FileCheck %s --check-prefixes=CHECK,CGSCC
 ;
 ;    void bar(int, float, double);

diff  --git a/llvm/test/Transforms/Attributor/align.ll b/llvm/test/Transforms/Attributor/align.ll
index 7bda25c18c103..7f548413d0549 100644
--- a/llvm/test/Transforms/Attributor/align.ll
+++ b/llvm/test/Transforms/Attributor/align.ll
@@ -157,17 +157,19 @@ define internal ptr @f1(ptr readnone %0) local_unnamed_addr #0 {
 }
 
 ; Function Attrs: nounwind readnone ssp uwtable
-define internal ptr @f2(ptr readnone %0) local_unnamed_addr #0 {
-; CGSCC: Function Attrs: noinline nounwind uwtable
-; CGSCC-LABEL: define {{[^@]+}}@f2
-; CGSCC-SAME: (ptr readnone [[TMP0:%.*]]) local_unnamed_addr #[[ATTR1:[0-9]+]] {
-; CGSCC-NEXT:    unreachable
-; CGSCC:       2:
-; CGSCC-NEXT:    unreachable
-; CGSCC:       3:
-; CGSCC-NEXT:    unreachable
-; CGSCC:       4:
-; CGSCC-NEXT:    unreachable
+define ptr @f2(ptr readnone %0) local_unnamed_addr #0 {
+; CHECK: Function Attrs: nofree noinline norecurse nosync nounwind willreturn memory(none) uwtable
+; CHECK-LABEL: define {{[^@]+}}@f2
+; CHECK-SAME: (ptr nofree readnone [[TMP0:%.*]]) local_unnamed_addr #[[ATTR0]] {
+; CHECK-NEXT:    [[TMP2:%.*]] = icmp eq ptr [[TMP0]], null
+; CHECK-NEXT:    br i1 [[TMP2]], label [[TMP4:%.*]], label [[TMP3:%.*]]
+; CHECK:       3:
+; CHECK-NEXT:    br label [[TMP5:%.*]]
+; CHECK:       4:
+; CHECK-NEXT:    br label [[TMP5]]
+; CHECK:       5:
+; CHECK-NEXT:    [[TMP6:%.*]] = phi ptr [ [[TMP0]], [[TMP3]] ], [ @a1, [[TMP4]] ]
+; CHECK-NEXT:    ret ptr [[TMP6]]
 ;
   %2 = icmp eq i8* %0, null
   br i1 %2, label %5, label %3
@@ -188,14 +190,14 @@ define internal ptr @f2(ptr readnone %0) local_unnamed_addr #0 {
 
 ; Function Attrs: nounwind readnone ssp uwtable
 define internal ptr @f3(ptr readnone %0) local_unnamed_addr #0 {
-; CGSCC: Function Attrs: noinline nounwind uwtable
+; CGSCC: Function Attrs: nofree noinline norecurse nosync nounwind willreturn memory(none) uwtable
 ; CGSCC-LABEL: define {{[^@]+}}@f3
-; CGSCC-SAME: (ptr readnone [[TMP0:%.*]]) local_unnamed_addr #[[ATTR1]] {
+; CGSCC-SAME: () local_unnamed_addr #[[ATTR0]] {
+; CGSCC-NEXT:    br label [[TMP2:%.*]]
+; CGSCC:       1:
 ; CGSCC-NEXT:    unreachable
 ; CGSCC:       2:
-; CGSCC-NEXT:    unreachable
-; CGSCC:       3:
-; CGSCC-NEXT:    unreachable
+; CGSCC-NEXT:    ret ptr undef
 ;
   %2 = icmp eq i8* %0, null
   br i1 %2, label %3, label %5
@@ -219,7 +221,7 @@ define align 4 ptr @test7() #0 {
 ;
 ; CGSCC: Function Attrs: nofree noinline nosync nounwind willreturn memory(none) uwtable
 ; CGSCC-LABEL: define {{[^@]+}}@test7
-; CGSCC-SAME: () #[[ATTR2:[0-9]+]] {
+; CGSCC-SAME: () #[[ATTR1:[0-9]+]] {
 ; CGSCC-NEXT:    [[C:%.*]] = tail call noundef nonnull align 8 dereferenceable(1) ptr @f1() #[[ATTR13:[0-9]+]]
 ; CGSCC-NEXT:    ret ptr [[C]]
 ;
@@ -258,7 +260,7 @@ define internal ptr @f2b(ptr readnone %0) local_unnamed_addr #0 {
 ;
 ; CGSCC: Function Attrs: noinline nounwind uwtable
 ; CGSCC-LABEL: define {{[^@]+}}@f2b
-; CGSCC-SAME: (ptr readnone [[TMP0:%.*]]) local_unnamed_addr #[[ATTR1]] {
+; CGSCC-SAME: (ptr readnone [[TMP0:%.*]]) local_unnamed_addr #[[ATTR2:[0-9]+]] {
 ; CGSCC-NEXT:    unreachable
 ; CGSCC:       2:
 ; CGSCC-NEXT:    unreachable
@@ -289,7 +291,7 @@ define internal ptr @f3b(ptr readnone %0) local_unnamed_addr #0 {
 ;
 ; CGSCC: Function Attrs: noinline nounwind uwtable
 ; CGSCC-LABEL: define {{[^@]+}}@f3b
-; CGSCC-SAME: (ptr readnone [[TMP0:%.*]]) local_unnamed_addr #[[ATTR1]] {
+; CGSCC-SAME: (ptr readnone [[TMP0:%.*]]) local_unnamed_addr #[[ATTR2]] {
 ; CGSCC-NEXT:    unreachable
 ; CGSCC:       2:
 ; CGSCC-NEXT:    unreachable
@@ -316,7 +318,7 @@ define align 4 ptr @test7b(ptr align 32 %p) #0 {
 ;
 ; CGSCC: Function Attrs: nofree noinline nosync nounwind willreturn memory(none) uwtable
 ; CGSCC-LABEL: define {{[^@]+}}@test7b
-; CGSCC-SAME: (ptr nofree readnone returned align 32 "no-capture-maybe-returned" [[P:%.*]]) #[[ATTR2]] {
+; CGSCC-SAME: (ptr nofree readnone returned align 32 "no-capture-maybe-returned" [[P:%.*]]) #[[ATTR1]] {
 ; CGSCC-NEXT:    ret ptr [[P]]
 ;
   tail call i8* @f1b(i8* align 8 dereferenceable(1) @a1)
@@ -1101,8 +1103,8 @@ attributes #2 = { null_pointer_is_valid }
 ; TUNIT: attributes #[[ATTR11]] = { nofree nosync nounwind willreturn }
 ;.
 ; CGSCC: attributes #[[ATTR0]] = { nofree noinline norecurse nosync nounwind willreturn memory(none) uwtable }
-; CGSCC: attributes #[[ATTR1]] = { noinline nounwind uwtable }
-; CGSCC: attributes #[[ATTR2]] = { nofree noinline nosync nounwind willreturn memory(none) uwtable }
+; CGSCC: attributes #[[ATTR1]] = { nofree noinline nosync nounwind willreturn memory(none) uwtable }
+; CGSCC: attributes #[[ATTR2]] = { noinline nounwind uwtable }
 ; CGSCC: attributes #[[ATTR3]] = { nounwind }
 ; CGSCC: attributes #[[ATTR4]] = { nofree nosync nounwind }
 ; CGSCC: attributes #[[ATTR5]] = { nofree norecurse nosync nounwind willreturn memory(argmem: read) }

diff  --git a/llvm/test/Transforms/Attributor/value-simplify.ll b/llvm/test/Transforms/Attributor/value-simplify.ll
index 5471c8964bdb5..e01a8d84ed1f5 100644
--- a/llvm/test/Transforms/Attributor/value-simplify.ll
+++ b/llvm/test/Transforms/Attributor/value-simplify.ll
@@ -1317,6 +1317,52 @@ define internal i32 @ret_speculatable_expr(ptr %mem, i32 %a2) {
   ret i32 %add
 }
 
+define internal void @not_called1() {
+; CHECK: Function Attrs: nofree norecurse nosync nounwind willreturn memory(none)
+; CHECK-LABEL: define {{[^@]+}}@not_called1
+; CHECK-SAME: () #[[ATTR1]] {
+; CHECK-NEXT:    ret void
+;
+  ret void
+}
+define internal void @not_called2() {
+; CHECK: Function Attrs: nofree norecurse nosync nounwind willreturn memory(none)
+; CHECK-LABEL: define {{[^@]+}}@not_called2
+; CHECK-SAME: () #[[ATTR1]] {
+; CHECK-NEXT:    ret void
+;
+  ret void
+}
+define internal void @not_called3() {
+; CHECK: Function Attrs: nofree norecurse nosync nounwind willreturn memory(none)
+; CHECK-LABEL: define {{[^@]+}}@not_called3
+; CHECK-SAME: () #[[ATTR1]] {
+; CHECK-NEXT:    ret void
+;
+  ret void
+}
+declare void @useFnDecl(ptr addrspace(42));
+define void @useFnDef(ptr addrspace(42) %arg) {
+; CHECK-LABEL: define {{[^@]+}}@useFnDef
+; CHECK-SAME: (ptr addrspace(42) [[ARG:%.*]]) {
+; CHECK-NEXT:    call void @useFnDecl(ptr addrspace(42) [[ARG]])
+; CHECK-NEXT:    ret void
+;
+  call void @useFnDecl(ptr addrspace(42) %arg)
+  ret void
+}
+define i1 @user_of_not_called() {
+; CHECK-LABEL: define {{[^@]+}}@user_of_not_called() {
+; CHECK-NEXT:    call void @useFnDecl(ptr addrspace(42) noundef addrspacecast (ptr @not_called1 to ptr addrspace(42)))
+; CHECK-NEXT:    call void @useFnDef(ptr addrspace(42) noundef addrspacecast (ptr @not_called2 to ptr addrspace(42)))
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq ptr addrspace(42) addrspacecast (ptr @not_called3 to ptr addrspace(42)), null
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+  call void @useFnDecl(ptr addrspace(42) addrspacecast (ptr @not_called1 to ptr addrspace(42)))
+  call void @useFnDef(ptr addrspace(42) addrspacecast (ptr @not_called2 to ptr addrspace(42)))
+  %cmp = icmp eq ptr addrspace(42) addrspacecast (ptr @not_called3 to ptr addrspace(42)), null
+  ret i1 %cmp
+}
 
 ;.
 ; TUNIT: attributes #[[ATTR0:[0-9]+]] = { nocallback nofree nosync nounwind willreturn }


        


More information about the llvm-commits mailing list