[llvm] ab98f2c - Revert "[InferAttributes] Materialize all infered attributes for declaration"
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 14 15:46:31 PDT 2021
Nico,
Can you point me to the builder which was still failing? I'd missed an
AMDGPU test, but that was fixed before your revert. (I think your
revert actually left tree in a broken state since you don't revert that
change btw.)
Philip
On 4/14/21 3:42 PM, Nico Weber via llvm-commits wrote:
> Author: Nico Weber
> Date: 2021-04-14T18:41:20-04:00
> New Revision: ab98f2c7129a52e216fd7e088b964cf4af27b0f2
>
> URL: https://github.com/llvm/llvm-project/commit/ab98f2c7129a52e216fd7e088b964cf4af27b0f2
> DIFF: https://github.com/llvm/llvm-project/commit/ab98f2c7129a52e216fd7e088b964cf4af27b0f2.diff
>
> LOG: Revert "[InferAttributes] Materialize all infered attributes for declaration"
>
> Breaks check-clang, see comments on D100400
>
> Also revert follow-up "[NFC] Move a recently added utility into a location to enable reuse"
>
> This reverts commit 3ce61fb6d697d49db471c7077b88b3b9ec9dec66.
> This reverts commit 61a85da88235983da565bda0160367461fa0f382.
>
> Added:
>
>
> Modified:
> llvm/include/llvm/Transforms/Utils/Local.h
> llvm/lib/Transforms/IPO/InferFunctionAttrs.cpp
> llvm/lib/Transforms/Utils/Local.cpp
> llvm/test/Other/cgscc-devirt-iteration.ll
> llvm/test/Transforms/InferFunctionAttrs/annotate.ll
> llvm/test/Transforms/LICM/strlen.ll
>
> Removed:
>
>
>
> ################################################################################
> diff --git a/llvm/include/llvm/Transforms/Utils/Local.h b/llvm/include/llvm/Transforms/Utils/Local.h
> index 8ab066f0b6a3..f7efeeb56fd3 100644
> --- a/llvm/include/llvm/Transforms/Utils/Local.h
> +++ b/llvm/include/llvm/Transforms/Utils/Local.h
> @@ -488,15 +488,6 @@ 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 30402f109f30..685f8f7d7a00 100644
> --- a/llvm/lib/Transforms/IPO/InferFunctionAttrs.cpp
> +++ b/llvm/lib/Transforms/IPO/InferFunctionAttrs.cpp
> @@ -15,7 +15,6 @@
> #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"
> @@ -26,14 +25,9 @@ static bool inferAllPrototypeAttributes(
>
> for (Function &F : M.functions())
> // We only infer things using the prototype and the name; we don't need
> - // 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()) {
> + // definitions.
> + 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 89e62fb94ec1..e285f8aa7a20 100644
> --- a/llvm/lib/Transforms/Utils/Local.cpp
> +++ b/llvm/lib/Transforms/Utils/Local.cpp
> @@ -3392,33 +3392,3 @@ 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/Other/cgscc-devirt-iteration.ll b/llvm/test/Other/cgscc-devirt-iteration.ll
> index 092c624442db..651b7f3b2853 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='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: 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: 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
> +; 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
> ;
> ; 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: nofree nosync readnone
> +; CHECK: Function Attrs: 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: nofree nosync readnone
> +; CHECK: Function Attrs: 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 05cf79b3874b..c71688ba3d15 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 mustprogress }
> +; CHECK-DAG: attributes [[NOFREE_NOUNWIND_WILLRETURN]] = { nofree nounwind willreturn }
> ; CHECK-DAG: attributes [[NOFREE_NOUNWIND]] = { nofree nounwind }
> -; 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 [[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 [[NOFREE_NOUNWIND_READONLY]] = { nofree nounwind readonly }
> -; 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 [[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 [[NOFREE]] = { nofree }
> -; CHECK-DAG: attributes [[WILLRETURN]] = { willreturn mustprogress }
> -; CHECK-DAG: attributes [[INACCESSIBLEMEMORARGONLY_NOFREE_NOUNWIND_WILLRETURN]] = { inaccessiblemem_or_argmemonly nofree nounwind willreturn mustprogress }
> +; CHECK-DAG: attributes [[WILLRETURN]] = { willreturn }
> +; CHECK-DAG: attributes [[INACCESSIBLEMEMORARGONLY_NOFREE_NOUNWIND_WILLRETURN]] = { inaccessiblemem_or_argmemonly nofree nounwind willreturn }
>
> ; CHECK-DARWIN-DAG: attributes [[ARGMEMONLY_NOFREE]] = { argmemonly nofree }
> -; CHECK-NVPTX-DAG: attributes [[NOFREE_NOUNWIND_READNONE]] = { nofree nosync nounwind readnone }
> +; CHECK-NVPTX-DAG: attributes [[NOFREE_NOUNWIND_READNONE]] = { nofree nounwind readnone }
>
> diff --git a/llvm/test/Transforms/LICM/strlen.ll b/llvm/test/Transforms/LICM/strlen.ll
> index 4a45a06f82f7..3f54eecafd80 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 mustprogress }
> +; CHECK: attributes #0 = { argmemonly nofree nounwind readonly willreturn }
> declare i64 @strlen(i8*)
>
>
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list