[clang] dd98555 - Reapply "[InferAttributes] Materialize all infered attributes for declaration"" and follow on patches.

Philip Reames via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 14 16:38:20 PDT 2021


Author: Philip Reames
Date: 2021-04-14T16:38:07-07:00
New Revision: dd985551c247752ee2be71f04a141225a40641ef

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

LOG: Reapply "[InferAttributes] Materialize all infered attributes for declaration"" and follow on patches.

This reverts commit ab98f2c7129a52e216fd7e088b964cf4af27b0f2 and 98eea392cdbcdb7360e58b46e9329573f092cd96.

It includes a fix for the clang test which triggered the revert.  I failed to notice this one because there was another AMDGPU llvm test with a similiar name and the exact same text in the error message.  Odd.  Since only one build bot reported the clang test, I didn't notice that one.

Added: 
    

Modified: 
    clang/test/CodeGenOpenCL/builtins-amdgcn.cl
    llvm/include/llvm/Transforms/Utils/Local.h
    llvm/lib/Transforms/IPO/InferFunctionAttrs.cpp
    llvm/lib/Transforms/Utils/Local.cpp
    llvm/test/CodeGen/AMDGPU/simplify-libcalls.ll
    llvm/test/Other/cgscc-devirt-iteration.ll
    llvm/test/Transforms/InferFunctionAttrs/annotate.ll
    llvm/test/Transforms/LICM/strlen.ll

Removed: 
    


################################################################################
diff  --git a/clang/test/CodeGenOpenCL/builtins-amdgcn.cl b/clang/test/CodeGenOpenCL/builtins-amdgcn.cl
index 3769149c8c6d..b02e6308c343 100644
--- a/clang/test/CodeGenOpenCL/builtins-amdgcn.cl
+++ b/clang/test/CodeGenOpenCL/builtins-amdgcn.cl
@@ -748,7 +748,7 @@ kernel void test_s_setreg(uint val) {
 
 // CHECK-DAG: [[$WI_RANGE]] = !{i32 0, i32 1024}
 // CHECK-DAG: [[$WS_RANGE]] = !{i16 1, i16 1025}
-// CHECK-DAG: attributes #[[$NOUNWIND_READONLY:[0-9]+]] = { nounwind readonly }
+// CHECK-DAG: attributes #[[$NOUNWIND_READONLY:[0-9]+]] = { nofree nounwind readonly }
 // CHECK-DAG: attributes #[[$READ_EXEC_ATTRS]] = { convergent }
 // CHECK-DAG: ![[$EXEC]] = !{!"exec"}
 // CHECK-DAG: ![[$EXEC_LO]] = !{!"exec_lo"}

diff  --git a/llvm/include/llvm/Transforms/Utils/Local.h b/llvm/include/llvm/Transforms/Utils/Local.h
index f7efeeb56fd3..8ab066f0b6a3 100644
--- a/llvm/include/llvm/Transforms/Utils/Local.h
+++ b/llvm/include/llvm/Transforms/Utils/Local.h
@@ -488,6 +488,15 @@ bool canReplaceOperandWithVariable(const Instruction *I, unsigned OpIdx);
 /// Invert the given true/false value, possibly reusing an existing copy.
 Value *invertCondition(Value *Condition);
 
+
+//===----------------------------------------------------------------------===//
+//  Assorted
+//
+
+/// If we can infer one attribute from another on the declaration of a
+/// function, explicitly materialize the maximal set in the IR.
+bool inferAttributesFromOthers(Function &F);
+
 } // end namespace llvm
 
 #endif // LLVM_TRANSFORMS_UTILS_LOCAL_H

diff  --git a/llvm/lib/Transforms/IPO/InferFunctionAttrs.cpp b/llvm/lib/Transforms/IPO/InferFunctionAttrs.cpp
index 685f8f7d7a00..30402f109f30 100644
--- a/llvm/lib/Transforms/IPO/InferFunctionAttrs.cpp
+++ b/llvm/lib/Transforms/IPO/InferFunctionAttrs.cpp
@@ -15,6 +15,7 @@
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/Utils/BuildLibCalls.h"
+#include "llvm/Transforms/Utils/Local.h"
 using namespace llvm;
 
 #define DEBUG_TYPE "inferattrs"
@@ -25,9 +26,14 @@ static bool inferAllPrototypeAttributes(
 
   for (Function &F : M.functions())
     // We only infer things using the prototype and the name; we don't need
-    // definitions.
-    if (F.isDeclaration() && !F.hasOptNone())
+    // definitions.  This ensures libfuncs are annotated and also allows our
+    // CGSCC inference to avoid needing to duplicate the inference from other
+    // attribute logic on all calls to declarations (as declarations aren't
+    // explicitly visited by CGSCC passes in the new pass manager.)
+    if (F.isDeclaration() && !F.hasOptNone()) {
       Changed |= inferLibFuncAttributes(F, GetTLI(F));
+      Changed |= inferAttributesFromOthers(F);
+    }
 
   return Changed;
 }

diff  --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index e285f8aa7a20..89e62fb94ec1 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -3392,3 +3392,33 @@ Value *llvm::invertCondition(Value *Condition) {
     Inverted->insertBefore(&*Parent->getFirstInsertionPt());
   return Inverted;
 }
+
+bool llvm::inferAttributesFromOthers(Function &F) {
+  // Note: We explicitly check for attributes rather than using cover functions
+  // because some of the cover functions include the logic being implemented.
+
+  bool Changed = false;
+  // readnone + not convergent implies nosync
+  if (!F.hasFnAttribute(Attribute::NoSync) &&
+      F.doesNotAccessMemory() && !F.isConvergent()) {
+    F.setNoSync();
+    Changed = true;
+  }
+
+  // readonly implies nofree
+  if (!F.hasFnAttribute(Attribute::NoFree) && F.onlyReadsMemory()) {
+    F.setDoesNotFreeMemory();
+    Changed = true;
+  }
+
+  // willreturn implies mustprogress
+  if (!F.hasFnAttribute(Attribute::MustProgress) && F.willReturn()) {
+    F.setMustProgress();
+    Changed = true;
+  }
+
+  // TODO: There are a bunch of cases of restrictive memory effects we
+  // can infer by inspecting arguments of argmemonly-ish functions.
+
+  return Changed;
+}

diff  --git a/llvm/test/CodeGen/AMDGPU/simplify-libcalls.ll b/llvm/test/CodeGen/AMDGPU/simplify-libcalls.ll
index 2ae332ffa437..1919f87a3a6f 100644
--- a/llvm/test/CodeGen/AMDGPU/simplify-libcalls.ll
+++ b/llvm/test/CodeGen/AMDGPU/simplify-libcalls.ll
@@ -791,5 +791,5 @@ entry:
 ; GCN-PRELINK: declare float @_Z11native_sqrtf(float) local_unnamed_addr #[[$NOUNWIND_READONLY]]
 
 ; GCN-PRELINK: attributes #[[$NOUNWIND]] = { nounwind }
-; GCN-PRELINK: attributes #[[$NOUNWIND_READONLY]] = { nounwind readonly }
+; GCN-PRELINK: attributes #[[$NOUNWIND_READONLY]] = { nofree nounwind readonly }
 attributes #0 = { nounwind }

diff  --git a/llvm/test/Other/cgscc-devirt-iteration.ll b/llvm/test/Other/cgscc-devirt-iteration.ll
index 651b7f3b2853..092c624442db 100644
--- a/llvm/test/Other/cgscc-devirt-iteration.ll
+++ b/llvm/test/Other/cgscc-devirt-iteration.ll
@@ -4,18 +4,18 @@
 ; devirtualization here with GVN which forwards a store through a load and to
 ; an indirect call.
 ;
-; RUN: opt -aa-pipeline=basic-aa -passes='cgscc(function-attrs,function(gvn,instcombine))' -S < %s | FileCheck %s --check-prefix=CHECK --check-prefix=BEFORE
-; 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
-; 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
+; 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
+; 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
+; 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
 ;
-; 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
-; RUN: opt -abort-on-max-devirt-iterations-reached -aa-pipeline=basic-aa -passes='cgscc(devirt<2>(function-attrs,function(gvn,instcombine)))' -S < %s
+; 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
+; 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
 ;
 ; We also verify that the real O2 pipeline catches these cases.
 ; RUN: opt -aa-pipeline=basic-aa -passes='default<O2>' -S < %s | FileCheck %s --check-prefix=CHECK --check-prefix=AFTER --check-prefix=AFTER2
 
 declare void @readnone() readnone
-; CHECK: Function Attrs: readnone
+; CHECK: Function Attrs: nofree nosync readnone
 ; CHECK-NEXT: declare void @readnone()
 
 declare void @unknown()
@@ -51,7 +51,7 @@ entry:
 ; devirtualize again, and then deduce readnone.
 
 declare void @readnone_with_arg(void ()**) readnone
-; CHECK: Function Attrs: readnone
+; CHECK: Function Attrs: nofree nosync readnone
 ; CHECK-LABEL: declare void @readnone_with_arg(void ()**)
 
 define void @test2_a(void ()** %ignore) {

diff  --git a/llvm/test/Transforms/InferFunctionAttrs/annotate.ll b/llvm/test/Transforms/InferFunctionAttrs/annotate.ll
index c71688ba3d15..05cf79b3874b 100644
--- a/llvm/test/Transforms/InferFunctionAttrs/annotate.ll
+++ b/llvm/test/Transforms/InferFunctionAttrs/annotate.ll
@@ -1016,18 +1016,18 @@ declare i64 @write(i32, i8*, i64)
 declare void @memset_pattern16(i8*, i8*, i64)
 
 
-; CHECK-DAG: attributes [[NOFREE_NOUNWIND_WILLRETURN]] = { nofree nounwind willreturn }
+; CHECK-DAG: attributes [[NOFREE_NOUNWIND_WILLRETURN]] = { nofree nounwind willreturn mustprogress }
 ; CHECK-DAG: attributes [[NOFREE_NOUNWIND]] = { nofree nounwind }
-; CHECK-DAG: attributes [[INACCESSIBLEMEMONLY_NOFREE_NOUNWIND_WILLRETURN]] = { inaccessiblememonly nofree nounwind willreturn }
-; CHECK-DAG: attributes [[NOFREE_NOUNWIND_READONLY_WILLRETURN]] = { nofree nounwind readonly willreturn }
-; CHECK-DAG: attributes [[ARGMEMONLY_NOFREE_NOUNWIND_WILLRETURN]] = { argmemonly nofree nounwind willreturn }
+; CHECK-DAG: attributes [[INACCESSIBLEMEMONLY_NOFREE_NOUNWIND_WILLRETURN]] = { inaccessiblememonly nofree nounwind willreturn mustprogress }
+; CHECK-DAG: attributes [[NOFREE_NOUNWIND_READONLY_WILLRETURN]] = { nofree nounwind readonly willreturn mustprogress }
+; CHECK-DAG: attributes [[ARGMEMONLY_NOFREE_NOUNWIND_WILLRETURN]] = { argmemonly nofree nounwind willreturn mustprogress }
 ; CHECK-DAG: attributes [[NOFREE_NOUNWIND_READONLY]] = { nofree nounwind readonly }
-; CHECK-DAG: attributes [[INACCESSIBLEMEMORARGMEMONLY_NOUNWIND_WILLRETURN]] = { inaccessiblemem_or_argmemonly nounwind willreturn }
-; CHECK-DAG: attributes [[NOFREE_WILLRETURN]] = { nofree willreturn }
-; CHECK-DAG: attributes [[ARGMEMONLY_NOFREE_NOUNWIND_READONLY_WILLRETURN]] = { argmemonly nofree nounwind readonly willreturn }
+; CHECK-DAG: attributes [[INACCESSIBLEMEMORARGMEMONLY_NOUNWIND_WILLRETURN]] = { inaccessiblemem_or_argmemonly nounwind willreturn mustprogress }
+; CHECK-DAG: attributes [[NOFREE_WILLRETURN]] = { nofree willreturn mustprogress }
+; CHECK-DAG: attributes [[ARGMEMONLY_NOFREE_NOUNWIND_READONLY_WILLRETURN]] = { argmemonly nofree nounwind readonly willreturn mustprogress }
 ; CHECK-DAG: attributes [[NOFREE]] = { nofree }
-; CHECK-DAG: attributes [[WILLRETURN]] = { willreturn }
-; CHECK-DAG: attributes [[INACCESSIBLEMEMORARGONLY_NOFREE_NOUNWIND_WILLRETURN]]  = { inaccessiblemem_or_argmemonly nofree nounwind willreturn }
+; CHECK-DAG: attributes [[WILLRETURN]] = { willreturn mustprogress }
+; CHECK-DAG: attributes [[INACCESSIBLEMEMORARGONLY_NOFREE_NOUNWIND_WILLRETURN]]  = { inaccessiblemem_or_argmemonly nofree nounwind willreturn mustprogress }
 
 ; CHECK-DARWIN-DAG: attributes [[ARGMEMONLY_NOFREE]] = { argmemonly nofree }
-; CHECK-NVPTX-DAG: attributes [[NOFREE_NOUNWIND_READNONE]] = { nofree nounwind readnone }
+; CHECK-NVPTX-DAG: attributes [[NOFREE_NOUNWIND_READNONE]] = { nofree nosync nounwind readnone }

diff  --git a/llvm/test/Transforms/LICM/strlen.ll b/llvm/test/Transforms/LICM/strlen.ll
index 3f54eecafd80..4a45a06f82f7 100644
--- a/llvm/test/Transforms/LICM/strlen.ll
+++ b/llvm/test/Transforms/LICM/strlen.ll
@@ -13,7 +13,7 @@ loop:
 }
 
 ; CHECK: declare i64 @strlen(i8* nocapture) #0
-; CHECK: attributes #0 = { argmemonly nofree nounwind readonly willreturn }
+; CHECK: attributes #0 = { argmemonly nofree nounwind readonly willreturn mustprogress }
 declare i64 @strlen(i8*)
 
 


        


More information about the cfe-commits mailing list