[llvm] 2543567 - [AssumeBundles] filter usefull attriutes to preserve

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 13 11:37:31 PDT 2020


Style: If something is necessary for correctness, call the function 
something other than "useful".  That reads as profitability, not legality.

Philip

p.s. Most of these attributes (deref, nonnull, etc..) don't need assumes 
in the common case where we're returning a value we can put metadata 
on.  See https://reviews.llvm.org/D76140 for another take on this problem.

On 3/13/20 9:36 AM, via llvm-commits wrote:
> Author: Tyker
> Date: 2020-03-13T17:35:47+01:00
> New Revision: 2543567c414fb82b921485b9923d9ad7e0c7c22b
>
> URL: https://github.com/llvm/llvm-project/commit/2543567c414fb82b921485b9923d9ad7e0c7c22b
> DIFF: https://github.com/llvm/llvm-project/commit/2543567c414fb82b921485b9923d9ad7e0c7c22b.diff
>
> LOG: [AssumeBundles] filter usefull attriutes to preserve
>
> Summary:
> This patch will filter attributes to only preserve those that are usefull.
> In the case of NoAlias it is filtered out not because it isn't usefull
> but because it is incorrect to preserve it as it is only valdi for the
> duration of the function.
>
> Reviewers: jdoerfert
>
> Reviewed By: jdoerfert
>
> Subscribers: jdoerfert, hiraditya, llvm-commits
>
> Tags: #llvm
>
> Differential Revision: https://reviews.llvm.org/D75828
>
> Added:
>      
>
> Modified:
>      llvm/lib/IR/KnowledgeRetention.cpp
>      llvm/test/IR/assume-builder.ll
>      llvm/test/Transforms/Inline/noalias2.ll
>      llvm/unittests/IR/KnowledgeRetentionTest.cpp
>
> Removed:
>      
>
>
> ################################################################################
> diff  --git a/llvm/lib/IR/KnowledgeRetention.cpp b/llvm/lib/IR/KnowledgeRetention.cpp
> index 66c92ea5798f..ba3644f08aa1 100644
> --- a/llvm/lib/IR/KnowledgeRetention.cpp
> +++ b/llvm/lib/IR/KnowledgeRetention.cpp
> @@ -95,6 +95,19 @@ bool isLowerOpBundle(const OperandBundleDef &LHS, const OperandBundleDef &RHS) {
>     return getTuple(LHS) < getTuple(RHS);
>   }
>   
> +bool isUsefullToPreserve(Attribute::AttrKind Kind) {
> +  switch (Kind) {
> +    case Attribute::NonNull:
> +    case Attribute::Alignment:
> +    case Attribute::Dereferenceable:
> +    case Attribute::DereferenceableOrNull:
> +    case Attribute::Cold:
> +      return true;
> +    default:
> +      return false;
> +  }
> +}
> +
>   /// This class contain all knowledge that have been gather while building an
>   /// llvm.assume and the function to manipulate it.
>   struct AssumeBuilderState {
> @@ -105,13 +118,14 @@ struct AssumeBuilderState {
>     AssumeBuilderState(Module *M) : M(M) {}
>   
>     void addAttribute(Attribute Attr, Value *WasOn) {
> +    if (!ShouldPreserveAllAttributes &&
> +        (Attr.isTypeAttribute() || Attr.isStringAttribute() ||
> +         !isUsefullToPreserve(Attr.getKindAsEnum())))
> +      return;
>       StringRef Name;
>       Value *AttrArg = nullptr;
>       if (Attr.isStringAttribute())
> -      if (ShouldPreserveAllAttributes)
> -        Name = Attr.getKindAsString();
> -      else
> -        return;
> +      Name = Attr.getKindAsString();
>       else
>         Name = Attribute::getNameFromAttrKind(Attr.getKindAsEnum());
>       if (Attr.isIntAttribute())
> @@ -127,9 +141,8 @@ struct AssumeBuilderState {
>              Idx < AttrList.getNumAttrSets(); Idx++)
>           for (Attribute Attr : AttrList.getAttributes(Idx))
>             addAttribute(Attr, Call->getArgOperand(Idx - 1));
> -      if (ShouldPreserveAllAttributes)
> -        for (Attribute Attr : AttrList.getFnAttributes())
> -          addAttribute(Attr, nullptr);
> +      for (Attribute Attr : AttrList.getFnAttributes())
> +        addAttribute(Attr, nullptr);
>       };
>       addAttrList(Call->getAttributes());
>       if (Function *Fn = Call->getCalledFunction())
>
> diff  --git a/llvm/test/IR/assume-builder.ll b/llvm/test/IR/assume-builder.ll
> index 54e382ccd4e7..955be3a8225c 100644
> --- a/llvm/test/IR/assume-builder.ll
> +++ b/llvm/test/IR/assume-builder.ll
> @@ -14,9 +14,9 @@ define void @test(i32* %P, i32* %P1, i32* %P2, i32* %P3) {
>   ; BASIC-NEXT:    call void @func(i32* nonnull dereferenceable(16) [[P]], i32* null)
>   ; BASIC-NEXT:    call void @llvm.assume(i1 true) [ "dereferenceable"(i32* [[P1:%.*]], i64 12), "nonnull"(i32* [[P]]) ]
>   ; BASIC-NEXT:    call void @func(i32* dereferenceable(12) [[P1]], i32* nonnull [[P]])
> -; BASIC-NEXT:    call void @llvm.assume(i1 true) [ "dereferenceable"(i32* [[P1]], i64 12) ]
> +; BASIC-NEXT:    call void @llvm.assume(i1 true) [ "cold"(), "dereferenceable"(i32* [[P1]], i64 12) ]
>   ; BASIC-NEXT:    call void @func_cold(i32* dereferenceable(12) [[P1]]) #0
> -; BASIC-NEXT:    call void @llvm.assume(i1 true) [ "dereferenceable"(i32* [[P1]], i64 12) ]
> +; BASIC-NEXT:    call void @llvm.assume(i1 true) [ "cold"(), "dereferenceable"(i32* [[P1]], i64 12) ]
>   ; BASIC-NEXT:    call void @func_cold(i32* dereferenceable(12) [[P1]])
>   ; BASIC-NEXT:    call void @func(i32* [[P1]], i32* [[P]])
>   ; BASIC-NEXT:    call void @func_strbool(i32* [[P1]])
>
> diff  --git a/llvm/test/Transforms/Inline/noalias2.ll b/llvm/test/Transforms/Inline/noalias2.ll
> index 79e7c0089397..f2b57e90eb5d 100644
> --- a/llvm/test/Transforms/Inline/noalias2.ll
> +++ b/llvm/test/Transforms/Inline/noalias2.ll
> @@ -28,29 +28,27 @@ entry:
>   }
>   
>   define void @foo(float* noalias nocapture %a, float* noalias nocapture readonly %c) #0 {
> -; NO_ASSUME-LABEL: define {{[^@]+}}@foo
> -; NO_ASSUME-SAME: (float* noalias nocapture [[A:%.*]], float* noalias nocapture readonly [[C:%.*]]) #0
> -; NO_ASSUME-NEXT:  entry:
> -; NO_ASSUME-NEXT:    [[TMP0:%.*]] = load float, float* [[C]], align 4, !alias.scope !0, !noalias !3
> -; NO_ASSUME-NEXT:    [[ARRAYIDX_I:%.*]] = getelementptr inbounds float, float* [[A]], i64 5
> -; NO_ASSUME-NEXT:    store float [[TMP0]], float* [[ARRAYIDX_I]], align 4, !alias.scope !3, !noalias !0
> -; NO_ASSUME-NEXT:    [[TMP1:%.*]] = load float, float* [[C]], align 4
> -; NO_ASSUME-NEXT:    [[ARRAYIDX:%.*]] = getelementptr inbounds float, float* [[A]], i64 7
> -; NO_ASSUME-NEXT:    store float [[TMP1]], float* [[ARRAYIDX]], align 4
> -; NO_ASSUME-NEXT:    ret void
> -;
> -; USE_ASSUME-LABEL: define {{[^@]+}}@foo
> -; USE_ASSUME-SAME: (float* noalias nocapture [[A:%.*]], float* noalias nocapture readonly [[C:%.*]]) #0
> -; USE_ASSUME-NEXT:  entry:
> -; USE_ASSUME-NEXT:    call void @llvm.assume(i1 true) [ "noalias"(float* [[A]]), "noalias"(float* [[C]]), "nocapture"(float* [[A]]), "nocapture"(float* [[C]]), "readonly"(float* [[C]]) ]
> -; USE_ASSUME-NEXT:    [[TMP0:%.*]] = load float, float* [[C]], align 4, !alias.scope !0, !noalias !3
> -; USE_ASSUME-NEXT:    [[ARRAYIDX_I:%.*]] = getelementptr inbounds float, float* [[A]], i64 5
> -; USE_ASSUME-NEXT:    store float [[TMP0]], float* [[ARRAYIDX_I]], align 4, !alias.scope !3, !noalias !0
> -; USE_ASSUME-NEXT:    [[TMP1:%.*]] = load float, float* [[C]], align 4
> -; USE_ASSUME-NEXT:    [[ARRAYIDX:%.*]] = getelementptr inbounds float, float* [[A]], i64 7
> -; USE_ASSUME-NEXT:    store float [[TMP1]], float* [[ARRAYIDX]], align 4
> -; USE_ASSUME-NEXT:    ret void
> +; CHECK-LABEL: define {{[^@]+}}@foo
> +; CHECK-SAME: (float* noalias nocapture [[A:%.*]], float* noalias nocapture readonly [[C:%.*]]) #0
> +; CHECK-NEXT:  entry:
> +; CHECK-NEXT:    [[TMP0:%.*]] = load float, float* [[C]], align 4, !alias.scope !0, !noalias !3
> +; CHECK-NEXT:    [[ARRAYIDX_I:%.*]] = getelementptr inbounds float, float* [[A]], i64 5
> +; CHECK-NEXT:    store float [[TMP0]], float* [[ARRAYIDX_I]], align 4, !alias.scope !3, !noalias !0
> +; CHECK-NEXT:    [[TMP1:%.*]] = load float, float* [[C]], align 4
> +; CHECK-NEXT:    [[ARRAYIDX:%.*]] = getelementptr inbounds float, float* [[A]], i64 7
> +; CHECK-NEXT:    store float [[TMP1]], float* [[ARRAYIDX]], align 4
> +; CHECK-NEXT:    ret void
>   ;
> +; ASSUME-LABEL: @foo(
> +; ASSUME-NEXT:  entry:
> +; ASSUME-NEXT:    call void @llvm.assume(i1 true) [ "noalias"(float* [[A:%.*]]), "noalias"(float* [[C:%.*]]) ]
> +; ASSUME-NEXT:    [[TMP0:%.*]] = load float, float* [[C]], align 4, !alias.scope !0, !noalias !3
> +; ASSUME-NEXT:    [[ARRAYIDX_I:%.*]] = getelementptr inbounds float, float* [[A]], i64 5
> +; ASSUME-NEXT:    store float [[TMP0]], float* [[ARRAYIDX_I]], align 4, !alias.scope !3, !noalias !0
> +; ASSUME-NEXT:    [[TMP1:%.*]] = load float, float* [[C]], align 4
> +; ASSUME-NEXT:    [[ARRAYIDX:%.*]] = getelementptr inbounds float, float* [[A]], i64 7
> +; ASSUME-NEXT:    store float [[TMP1]], float* [[ARRAYIDX]], align 4
> +; ASSUME-NEXT:    ret void
>   entry:
>     tail call void @hello(float* %a, float* %c)
>     %0 = load float, float* %c, align 4
> @@ -82,46 +80,24 @@ entry:
>   ; Check that when hello() is inlined into foo(), and then foo() is inlined into
>   ; foo2(), the noalias scopes are properly concatenated.
>   define void @foo2(float* nocapture %a, float* nocapture %b, float* nocapture readonly %c) #0 {
> -; NO_ASSUME-LABEL: define {{[^@]+}}@foo2
> -; NO_ASSUME-SAME: (float* nocapture [[A:%.*]], float* nocapture [[B:%.*]], float* nocapture readonly [[C:%.*]]) #0
> -; NO_ASSUME-NEXT:  entry:
> -; NO_ASSUME-NEXT:    [[TMP0:%.*]] = load float, float* [[C]], align 4, !alias.scope !5, !noalias !10
> -; NO_ASSUME-NEXT:    [[ARRAYIDX_I_I:%.*]] = getelementptr inbounds float, float* [[A]], i64 5
> -; NO_ASSUME-NEXT:    store float [[TMP0]], float* [[ARRAYIDX_I_I]], align 4, !alias.scope !10, !noalias !5
> -; NO_ASSUME-NEXT:    [[TMP1:%.*]] = load float, float* [[C]], align 4, !alias.scope !13, !noalias !14
> -; NO_ASSUME-NEXT:    [[ARRAYIDX_I:%.*]] = getelementptr inbounds float, float* [[A]], i64 7
> -; NO_ASSUME-NEXT:    store float [[TMP1]], float* [[ARRAYIDX_I]], align 4, !alias.scope !14, !noalias !13
> -; NO_ASSUME-NEXT:    [[TMP2:%.*]] = load float, float* [[C]], align 4, !noalias !15
> -; NO_ASSUME-NEXT:    [[ARRAYIDX_I1:%.*]] = getelementptr inbounds float, float* [[A]], i64 6
> -; NO_ASSUME-NEXT:    store float [[TMP2]], float* [[ARRAYIDX_I1]], align 4, !alias.scope !19, !noalias !20
> -; NO_ASSUME-NEXT:    [[ARRAYIDX1_I:%.*]] = getelementptr inbounds float, float* [[B]], i64 8
> -; NO_ASSUME-NEXT:    store float [[TMP2]], float* [[ARRAYIDX1_I]], align 4, !alias.scope !20, !noalias !19
> -; NO_ASSUME-NEXT:    [[TMP3:%.*]] = load float, float* [[C]], align 4
> -; NO_ASSUME-NEXT:    [[ARRAYIDX:%.*]] = getelementptr inbounds float, float* [[A]], i64 7
> -; NO_ASSUME-NEXT:    store float [[TMP3]], float* [[ARRAYIDX]], align 4
> -; NO_ASSUME-NEXT:    ret void
> -;
> -; USE_ASSUME-LABEL: define {{[^@]+}}@foo2
> -; USE_ASSUME-SAME: (float* nocapture [[A:%.*]], float* nocapture [[B:%.*]], float* nocapture readonly [[C:%.*]]) #0
> -; USE_ASSUME-NEXT:  entry:
> -; USE_ASSUME-NEXT:    call void @llvm.assume(i1 true) [ "noalias"(float* [[A]]), "noalias"(float* [[C]]), "nocapture"(float* [[A]]), "nocapture"(float* [[C]]), "readonly"(float* [[C]]) ]
> -; USE_ASSUME-NEXT:    call void @llvm.assume(i1 true) #2 [ "noalias"(float* [[A]]), "noalias"(float* [[C]]), "nocapture"(float* [[A]]), "nocapture"(float* [[C]]), "readonly"(float* [[C]]) ], !noalias !5
> -; USE_ASSUME-NEXT:    [[TMP0:%.*]] = load float, float* [[C]], align 4, !alias.scope !9, !noalias !12
> -; USE_ASSUME-NEXT:    [[ARRAYIDX_I_I:%.*]] = getelementptr inbounds float, float* [[A]], i64 5
> -; USE_ASSUME-NEXT:    store float [[TMP0]], float* [[ARRAYIDX_I_I]], align 4, !alias.scope !12, !noalias !9
> -; USE_ASSUME-NEXT:    [[TMP1:%.*]] = load float, float* [[C]], align 4, !alias.scope !14, !noalias !15
> -; USE_ASSUME-NEXT:    [[ARRAYIDX_I:%.*]] = getelementptr inbounds float, float* [[A]], i64 7
> -; USE_ASSUME-NEXT:    store float [[TMP1]], float* [[ARRAYIDX_I]], align 4, !alias.scope !15, !noalias !14
> -; USE_ASSUME-NEXT:    call void @llvm.assume(i1 true) [ "noalias"(float* [[A]]), "noalias"(float* [[B]]), "nocapture"(float* [[A]]), "nocapture"(float* [[B]]), "nocapture"(float* [[C]]), "readonly"(float* [[C]]) ]
> -; USE_ASSUME-NEXT:    [[TMP2:%.*]] = load float, float* [[C]], align 4, !noalias !16
> -; USE_ASSUME-NEXT:    [[ARRAYIDX_I1:%.*]] = getelementptr inbounds float, float* [[A]], i64 6
> -; USE_ASSUME-NEXT:    store float [[TMP2]], float* [[ARRAYIDX_I1]], align 4, !alias.scope !20, !noalias !21
> -; USE_ASSUME-NEXT:    [[ARRAYIDX1_I:%.*]] = getelementptr inbounds float, float* [[B]], i64 8
> -; USE_ASSUME-NEXT:    store float [[TMP2]], float* [[ARRAYIDX1_I]], align 4, !alias.scope !21, !noalias !20
> -; USE_ASSUME-NEXT:    [[TMP3:%.*]] = load float, float* [[C]], align 4
> -; USE_ASSUME-NEXT:    [[ARRAYIDX:%.*]] = getelementptr inbounds float, float* [[A]], i64 7
> -; USE_ASSUME-NEXT:    store float [[TMP3]], float* [[ARRAYIDX]], align 4
> -; USE_ASSUME-NEXT:    ret void
> +; CHECK-LABEL: define {{[^@]+}}@foo2
> +; CHECK-SAME: (float* nocapture [[A:%.*]], float* nocapture [[B:%.*]], float* nocapture readonly [[C:%.*]]) #0
> +; CHECK-NEXT:  entry:
> +; CHECK-NEXT:    [[TMP0:%.*]] = load float, float* [[C]], align 4, !alias.scope !5, !noalias !10
> +; CHECK-NEXT:    [[ARRAYIDX_I_I:%.*]] = getelementptr inbounds float, float* [[A]], i64 5
> +; CHECK-NEXT:    store float [[TMP0]], float* [[ARRAYIDX_I_I]], align 4, !alias.scope !10, !noalias !5
> +; CHECK-NEXT:    [[TMP1:%.*]] = load float, float* [[C]], align 4, !alias.scope !13, !noalias !14
> +; CHECK-NEXT:    [[ARRAYIDX_I:%.*]] = getelementptr inbounds float, float* [[A]], i64 7
> +; CHECK-NEXT:    store float [[TMP1]], float* [[ARRAYIDX_I]], align 4, !alias.scope !14, !noalias !13
> +; CHECK-NEXT:    [[TMP2:%.*]] = load float, float* [[C]], align 4, !noalias !15
> +; CHECK-NEXT:    [[ARRAYIDX_I1:%.*]] = getelementptr inbounds float, float* [[A]], i64 6
> +; CHECK-NEXT:    store float [[TMP2]], float* [[ARRAYIDX_I1]], align 4, !alias.scope !19, !noalias !20
> +; CHECK-NEXT:    [[ARRAYIDX1_I:%.*]] = getelementptr inbounds float, float* [[B]], i64 8
> +; CHECK-NEXT:    store float [[TMP2]], float* [[ARRAYIDX1_I]], align 4, !alias.scope !20, !noalias !19
> +; CHECK-NEXT:    [[TMP3:%.*]] = load float, float* [[C]], align 4
> +; CHECK-NEXT:    [[ARRAYIDX:%.*]] = getelementptr inbounds float, float* [[A]], i64 7
> +; CHECK-NEXT:    store float [[TMP3]], float* [[ARRAYIDX]], align 4
> +; CHECK-NEXT:    ret void
>   ;
>   entry:
>     tail call void @foo(float* %a, float* %c)
>
> diff  --git a/llvm/unittests/IR/KnowledgeRetentionTest.cpp b/llvm/unittests/IR/KnowledgeRetentionTest.cpp
> index f4a341f23c7c..62caeebba895 100644
> --- a/llvm/unittests/IR/KnowledgeRetentionTest.cpp
> +++ b/llvm/unittests/IR/KnowledgeRetentionTest.cpp
> @@ -40,7 +40,7 @@ static void RunTest(
>     }
>   }
>   
> -static void AssertMatchesExactlyAttributes(CallInst *Assume, Value *WasOn,
> +bool hasMatchesExactlyAttributes(CallInst *Assume, Value *WasOn,
>                                       StringRef AttrToMatch) {
>     Regex Reg(AttrToMatch);
>     SmallVector<StringRef, 1> Matches;
> @@ -50,20 +50,22 @@ static void AssertMatchesExactlyAttributes(CallInst *Assume, Value *WasOn,
>   #include "llvm/IR/Attributes.inc"
>          }) {
>       bool ShouldHaveAttr = Reg.match(Attr, &Matches) && Matches[0] == Attr;
> -    if (ShouldHaveAttr != hasAttributeInAssume(*Assume, WasOn, Attr)) {
> -      ASSERT_TRUE(false);
> -    }
> +    if (ShouldHaveAttr != hasAttributeInAssume(*Assume, WasOn, Attr))
> +      return false;
>     }
> +  return true;
>   }
>   
> -static void AssertHasTheRightValue(CallInst *Assume, Value *WasOn,
> +bool hasTheRightValue(CallInst *Assume, Value *WasOn,
>                               Attribute::AttrKind Kind, unsigned Value, bool Both,
>                               AssumeQuery AQ = AssumeQuery::Highest) {
>     if (!Both) {
>       uint64_t ArgVal = 0;
> -    ASSERT_TRUE(hasAttributeInAssume(*Assume, WasOn, Kind, &ArgVal, AQ));
> -    ASSERT_EQ(ArgVal, Value);
> -    return;
> +    if (!hasAttributeInAssume(*Assume, WasOn, Kind, &ArgVal, AQ))
> +      return false;
> +    if (ArgVal != Value)
> +      return false;
> +    return true;
>     }
>     uint64_t ArgValLow = 0;
>     uint64_t ArgValHigh = 0;
> @@ -71,12 +73,11 @@ static void AssertHasTheRightValue(CallInst *Assume, Value *WasOn,
>                                           AssumeQuery::Lowest);
>     bool ResultHigh = hasAttributeInAssume(*Assume, WasOn, Kind, &ArgValHigh,
>                                            AssumeQuery::Highest);
> -  if (ResultLow != ResultHigh || ResultHigh == false) {
> -    ASSERT_TRUE(false);
> -  }
> -  if (ArgValLow != Value || ArgValLow != ArgValHigh) {
> -    ASSERT_TRUE(false);
> -  }
> +  if (ResultLow != ResultHigh || ResultHigh == false)
> +    return false;
> +  if (ArgValLow != Value || ArgValLow != ArgValHigh)
> +    return false;
> +  return true;
>   }
>   
>   TEST(AssumeQueryAPI, hasAttributeInAssume) {
> @@ -98,16 +99,16 @@ TEST(AssumeQueryAPI, hasAttributeInAssume) {
>         [](Instruction *I) {
>           CallInst *Assume = BuildAssumeFromInst(I);
>           Assume->insertBefore(I);
> -        AssertMatchesExactlyAttributes(Assume, I->getOperand(0),
> -                                       "(nonnull|align|dereferenceable)");
> -        AssertMatchesExactlyAttributes(Assume, I->getOperand(1),
> -                                       "(noalias|align)");
> -        AssertHasTheRightValue(Assume, I->getOperand(0),
> -                               Attribute::AttrKind::Dereferenceable, 16, true);
> -        AssertHasTheRightValue(Assume, I->getOperand(0),
> -                               Attribute::AttrKind::Alignment, 4, true);
> -        AssertHasTheRightValue(Assume, I->getOperand(0),
> -                               Attribute::AttrKind::Alignment, 4, true);
> +        ASSERT_TRUE(hasMatchesExactlyAttributes(Assume, I->getOperand(0),
> +                                       "(nonnull|align|dereferenceable)"));
> +        ASSERT_TRUE(hasMatchesExactlyAttributes(Assume, I->getOperand(1),
> +                                       "(align)"));
> +        ASSERT_TRUE(hasTheRightValue(Assume, I->getOperand(0),
> +                               Attribute::AttrKind::Dereferenceable, 16, true));
> +        ASSERT_TRUE(hasTheRightValue(Assume, I->getOperand(0),
> +                               Attribute::AttrKind::Alignment, 4, true));
> +        ASSERT_TRUE(hasTheRightValue(Assume, I->getOperand(0),
> +                               Attribute::AttrKind::Alignment, 4, true));
>         }));
>     Tests.push_back(std::make_pair(
>         "call void @func1(i32* nonnull align 32 dereferenceable(48) %P, i32* "
> @@ -118,48 +119,48 @@ TEST(AssumeQueryAPI, hasAttributeInAssume) {
>         [](Instruction *I) {
>           CallInst *Assume = BuildAssumeFromInst(I);
>           Assume->insertBefore(I);
> -        AssertMatchesExactlyAttributes(Assume, I->getOperand(0),
> -                                       "(nonnull|align|dereferenceable)");
> -        AssertMatchesExactlyAttributes(Assume, I->getOperand(1),
> -                                       "(nonnull|align|dereferenceable)");
> -        AssertMatchesExactlyAttributes(Assume, I->getOperand(2),
> -                                       "(nonnull|align|dereferenceable)");
> -        AssertMatchesExactlyAttributes(Assume, I->getOperand(3),
> -                                       "(nonnull|align|dereferenceable)");
> -        AssertHasTheRightValue(Assume, I->getOperand(0),
> +        ASSERT_TRUE(hasMatchesExactlyAttributes(Assume, I->getOperand(0),
> +                                       "(nonnull|align|dereferenceable)"));
> +        ASSERT_TRUE(hasMatchesExactlyAttributes(Assume, I->getOperand(1),
> +                                       "(nonnull|align|dereferenceable)"));
> +        ASSERT_TRUE(hasMatchesExactlyAttributes(Assume, I->getOperand(2),
> +                                       "(nonnull|align|dereferenceable)"));
> +        ASSERT_TRUE(hasMatchesExactlyAttributes(Assume, I->getOperand(3),
> +                                       "(nonnull|align|dereferenceable)"));
> +        ASSERT_TRUE(hasTheRightValue(Assume, I->getOperand(0),
>                                  Attribute::AttrKind::Dereferenceable, 48, false,
> -                               AssumeQuery::Highest);
> -        AssertHasTheRightValue(Assume, I->getOperand(0),
> +                               AssumeQuery::Highest));
> +        ASSERT_TRUE(hasTheRightValue(Assume, I->getOperand(0),
>                                  Attribute::AttrKind::Alignment, 64, false,
> -                               AssumeQuery::Highest);
> -        AssertHasTheRightValue(Assume, I->getOperand(1),
> +                               AssumeQuery::Highest));
> +        ASSERT_TRUE(hasTheRightValue(Assume, I->getOperand(1),
>                                  Attribute::AttrKind::Alignment, 64, false,
> -                               AssumeQuery::Highest);
> -        AssertHasTheRightValue(Assume, I->getOperand(0),
> +                               AssumeQuery::Highest));
> +        ASSERT_TRUE(hasTheRightValue(Assume, I->getOperand(0),
>                                  Attribute::AttrKind::Dereferenceable, 4, false,
> -                               AssumeQuery::Lowest);
> -        AssertHasTheRightValue(Assume, I->getOperand(0),
> +                               AssumeQuery::Lowest));
> +        ASSERT_TRUE(hasTheRightValue(Assume, I->getOperand(0),
>                                  Attribute::AttrKind::Alignment, 8, false,
> -                               AssumeQuery::Lowest);
> -        AssertHasTheRightValue(Assume, I->getOperand(1),
> +                               AssumeQuery::Lowest));
> +        ASSERT_TRUE(hasTheRightValue(Assume, I->getOperand(1),
>                                  Attribute::AttrKind::Alignment, 8, false,
> -                               AssumeQuery::Lowest);
> +                               AssumeQuery::Lowest));
>         }));
>     Tests.push_back(std::make_pair(
>         "call void @func_many(i32* align 8 %P1) cold\n", [](Instruction *I) {
>           ShouldPreserveAllAttributes.setValue(true);
>           CallInst *Assume = BuildAssumeFromInst(I);
>           Assume->insertBefore(I);
> -        AssertMatchesExactlyAttributes(
> +        ASSERT_TRUE(hasMatchesExactlyAttributes(
>               Assume, nullptr,
>               "(align|no-jump-tables|less-precise-fpmad|"
> -            "nounwind|norecurse|willreturn|cold)");
> +            "nounwind|norecurse|willreturn|cold)"));
>           ShouldPreserveAllAttributes.setValue(false);
>         }));
>     Tests.push_back(
>         std::make_pair("call void @llvm.assume(i1 true)\n", [](Instruction *I) {
>           CallInst *Assume = cast<CallInst>(I);
> -        AssertMatchesExactlyAttributes(Assume, nullptr, "");
> +        ASSERT_TRUE(hasMatchesExactlyAttributes(Assume, nullptr, ""));
>         }));
>     Tests.push_back(std::make_pair(
>         "call void @func1(i32* readnone align 32 "
> @@ -170,31 +171,31 @@ TEST(AssumeQueryAPI, hasAttributeInAssume) {
>         [](Instruction *I) {
>           CallInst *Assume = BuildAssumeFromInst(I);
>           Assume->insertBefore(I);
> -        AssertMatchesExactlyAttributes(
> +        ASSERT_TRUE(hasMatchesExactlyAttributes(
>               Assume, I->getOperand(0),
> -            "(readnone|align|dereferenceable|noalias)");
> -        AssertMatchesExactlyAttributes(Assume, I->getOperand(1),
> -                                       "(align|dereferenceable)");
> -        AssertMatchesExactlyAttributes(Assume, I->getOperand(2),
> -                                       "(align|dereferenceable)");
> -        AssertMatchesExactlyAttributes(Assume, I->getOperand(3),
> -                                       "(nonnull|align|dereferenceable)");
> -        AssertHasTheRightValue(Assume, I->getOperand(0),
> -                               Attribute::AttrKind::Alignment, 32, true);
> -        AssertHasTheRightValue(Assume, I->getOperand(0),
> -                               Attribute::AttrKind::Dereferenceable, 48, true);
> -        AssertHasTheRightValue(Assume, I->getOperand(1),
> -                               Attribute::AttrKind::Dereferenceable, 28, true);
> -        AssertHasTheRightValue(Assume, I->getOperand(1),
> -                               Attribute::AttrKind::Alignment, 8, true);
> -        AssertHasTheRightValue(Assume, I->getOperand(2),
> -                               Attribute::AttrKind::Alignment, 64, true);
> -        AssertHasTheRightValue(Assume, I->getOperand(2),
> -                               Attribute::AttrKind::Dereferenceable, 4, true);
> -        AssertHasTheRightValue(Assume, I->getOperand(3),
> -                               Attribute::AttrKind::Alignment, 16, true);
> -        AssertHasTheRightValue(Assume, I->getOperand(3),
> -                               Attribute::AttrKind::Dereferenceable, 12, true);
> +            "(align|dereferenceable)"));
> +        ASSERT_TRUE(hasMatchesExactlyAttributes(Assume, I->getOperand(1),
> +                                       "(align|dereferenceable)"));
> +        ASSERT_TRUE(hasMatchesExactlyAttributes(Assume, I->getOperand(2),
> +                                       "(align|dereferenceable)"));
> +        ASSERT_TRUE(hasMatchesExactlyAttributes(Assume, I->getOperand(3),
> +                                       "(nonnull|align|dereferenceable)"));
> +        ASSERT_TRUE(hasTheRightValue(Assume, I->getOperand(0),
> +                               Attribute::AttrKind::Alignment, 32, true));
> +        ASSERT_TRUE(hasTheRightValue(Assume, I->getOperand(0),
> +                               Attribute::AttrKind::Dereferenceable, 48, true));
> +        ASSERT_TRUE(hasTheRightValue(Assume, I->getOperand(1),
> +                               Attribute::AttrKind::Dereferenceable, 28, true));
> +        ASSERT_TRUE(hasTheRightValue(Assume, I->getOperand(1),
> +                               Attribute::AttrKind::Alignment, 8, true));
> +        ASSERT_TRUE(hasTheRightValue(Assume, I->getOperand(2),
> +                               Attribute::AttrKind::Alignment, 64, true));
> +        ASSERT_TRUE(hasTheRightValue(Assume, I->getOperand(2),
> +                               Attribute::AttrKind::Dereferenceable, 4, true));
> +        ASSERT_TRUE(hasTheRightValue(Assume, I->getOperand(3),
> +                               Attribute::AttrKind::Alignment, 16, true));
> +        ASSERT_TRUE(hasTheRightValue(Assume, I->getOperand(3),
> +                               Attribute::AttrKind::Dereferenceable, 12, true));
>         }));
>   
>     Tests.push_back(std::make_pair(
> @@ -209,16 +210,19 @@ TEST(AssumeQueryAPI, hasAttributeInAssume) {
>           I->getOperand(1)->dropDroppableUses();
>           I->getOperand(2)->dropDroppableUses();
>           I->getOperand(3)->dropDroppableUses();
> -        AssertMatchesExactlyAttributes(
> +        ASSERT_TRUE(hasMatchesExactlyAttributes(
>               Assume, I->getOperand(0),
> -            "(readnone|align|dereferenceable|noalias)");
> -        AssertMatchesExactlyAttributes(Assume, I->getOperand(1), "");
> -        AssertMatchesExactlyAttributes(Assume, I->getOperand(2), "");
> -        AssertMatchesExactlyAttributes(Assume, I->getOperand(3), "");
> -        AssertHasTheRightValue(Assume, I->getOperand(0),
> -                               Attribute::AttrKind::Alignment, 32, true);
> -        AssertHasTheRightValue(Assume, I->getOperand(0),
> -                               Attribute::AttrKind::Dereferenceable, 48, true);
> +            "(align|dereferenceable)"));
> +        ASSERT_TRUE(hasMatchesExactlyAttributes(Assume, I->getOperand(1),
> +                                       ""));
> +        ASSERT_TRUE(hasMatchesExactlyAttributes(Assume, I->getOperand(2),
> +                                       ""));
> +        ASSERT_TRUE(hasMatchesExactlyAttributes(Assume, I->getOperand(3),
> +                                       ""));
> +        ASSERT_TRUE(hasTheRightValue(Assume, I->getOperand(0),
> +                               Attribute::AttrKind::Alignment, 32, true));
> +        ASSERT_TRUE(hasTheRightValue(Assume, I->getOperand(0),
> +                               Attribute::AttrKind::Dereferenceable, 48, true));
>         }));
>     Tests.push_back(std::make_pair(
>         "call void @func(i32* nonnull align 4 dereferenceable(16) %P, i32* align "
> @@ -228,18 +232,18 @@ TEST(AssumeQueryAPI, hasAttributeInAssume) {
>           Assume->insertBefore(I);
>           Value *New = I->getFunction()->getArg(3);
>           Value *Old = I->getOperand(0);
> -        AssertMatchesExactlyAttributes(Assume, New, "");
> -        AssertMatchesExactlyAttributes(Assume, Old,
> -                                       "(nonnull|align|dereferenceable)");
> +        ASSERT_TRUE(hasMatchesExactlyAttributes(Assume, New, ""));
> +        ASSERT_TRUE(hasMatchesExactlyAttributes(Assume, Old,
> +                                       "(nonnull|align|dereferenceable)"));
>           Old->replaceAllUsesWith(New);
> -        AssertMatchesExactlyAttributes(Assume, New,
> -                                       "(nonnull|align|dereferenceable)");
> -        AssertMatchesExactlyAttributes(Assume, Old, "");
> +        ASSERT_TRUE(hasMatchesExactlyAttributes(Assume, New,
> +                                       "(nonnull|align|dereferenceable)"));
> +        ASSERT_TRUE(hasMatchesExactlyAttributes(Assume, Old, ""));
>         }));
>     RunTest(Head, Tail, Tests);
>   }
>   
> -static void AssertFindExactlyAttributes(RetainedKnowledgeMap &Map, Value *WasOn,
> +static bool FindExactlyAttributes(RetainedKnowledgeMap &Map, Value *WasOn,
>                                    StringRef AttrToMatch) {
>     Regex Reg(AttrToMatch);
>     SmallVector<StringRef, 1> Matches;
> @@ -250,18 +254,17 @@ static void AssertFindExactlyAttributes(RetainedKnowledgeMap &Map, Value *WasOn,
>          }) {
>       bool ShouldHaveAttr = Reg.match(Attr, &Matches) && Matches[0] == Attr;
>   
> -    if (ShouldHaveAttr != (Map.find(RetainedKnowledgeKey{WasOn, Attribute::getAttrKindFromName(Attr)}) != Map.end())) {
> -      ASSERT_TRUE(false);
> -    }
> +    if (ShouldHaveAttr != (Map.find(RetainedKnowledgeKey{WasOn, Attribute::getAttrKindFromName(Attr)}) != Map.end()))
> +      return false;
>     }
> +  return true;
>   }
>   
> -static void AssertMapHasRightValue(RetainedKnowledgeMap &Map,
> +static bool MapHasRightValue(RetainedKnowledgeMap &Map,
>                                      RetainedKnowledgeKey Key, MinMax MM) {
>     auto LookupIt = Map.find(Key);
> -  ASSERT_TRUE(LookupIt != Map.end());
> -  ASSERT_TRUE(LookupIt->second.Min == MM.Min);
> -  ASSERT_TRUE(LookupIt->second.Max == MM.Max);
> +  return (LookupIt != Map.end()) && (LookupIt->second.Min == MM.Min) &&
> +         (LookupIt->second.Max == MM.Max);
>   }
>   
>   TEST(AssumeQueryAPI, fillMapFromAssume) {
> @@ -286,16 +289,16 @@ TEST(AssumeQueryAPI, fillMapFromAssume) {
>   
>           RetainedKnowledgeMap Map;
>           fillMapFromAssume(*Assume, Map);
> -        AssertFindExactlyAttributes(Map, I->getOperand(0),
> -                                       "(nonnull|align|dereferenceable)");
> -        AssertFindExactlyAttributes(Map, I->getOperand(1),
> -                                       "(noalias|align)");
> -        AssertMapHasRightValue(
> -            Map, {I->getOperand(0), Attribute::Dereferenceable}, {16, 16});
> -        AssertMapHasRightValue(Map, {I->getOperand(0), Attribute::Alignment},
> -                               {4, 4});
> -        AssertMapHasRightValue(Map, {I->getOperand(0), Attribute::Alignment},
> -                               {4, 4});
> +        ASSERT_TRUE(FindExactlyAttributes(Map, I->getOperand(0),
> +                                       "(nonnull|align|dereferenceable)"));
> +        ASSERT_TRUE(FindExactlyAttributes(Map, I->getOperand(1),
> +                                       "(align)"));
> +        ASSERT_TRUE(MapHasRightValue(
> +            Map, {I->getOperand(0), Attribute::Dereferenceable}, {16, 16}));
> +        ASSERT_TRUE(MapHasRightValue(Map, {I->getOperand(0), Attribute::Alignment},
> +                               {4, 4}));
> +        ASSERT_TRUE(MapHasRightValue(Map, {I->getOperand(0), Attribute::Alignment},
> +                               {4, 4}));
>         }));
>     Tests.push_back(std::make_pair(
>         "call void @func1(i32* nonnull align 32 dereferenceable(48) %P, i32* "
> @@ -310,18 +313,18 @@ TEST(AssumeQueryAPI, fillMapFromAssume) {
>           RetainedKnowledgeMap Map;
>           fillMapFromAssume(*Assume, Map);
>   
> -        AssertFindExactlyAttributes(Map, I->getOperand(0),
> -                                       "(nonnull|align|dereferenceable)");
> -        AssertFindExactlyAttributes(Map, I->getOperand(1),
> -                                       "(nonnull|align|dereferenceable)");
> -        AssertFindExactlyAttributes(Map, I->getOperand(2),
> -                                       "(nonnull|align|dereferenceable)");
> -        AssertFindExactlyAttributes(Map, I->getOperand(3),
> -                                       "(nonnull|align|dereferenceable)");
> -        AssertMapHasRightValue(
> -            Map, {I->getOperand(0), Attribute::Dereferenceable}, {4, 48});
> -        AssertMapHasRightValue(Map, {I->getOperand(0), Attribute::Alignment},
> -                               {8, 64});
> +        ASSERT_TRUE(FindExactlyAttributes(Map, I->getOperand(0),
> +                                       "(nonnull|align|dereferenceable)"));
> +        ASSERT_TRUE(FindExactlyAttributes(Map, I->getOperand(1),
> +                                       "(nonnull|align|dereferenceable)"));
> +        ASSERT_TRUE(FindExactlyAttributes(Map, I->getOperand(2),
> +                                       "(nonnull|align|dereferenceable)"));
> +        ASSERT_TRUE(FindExactlyAttributes(Map, I->getOperand(3),
> +                                       "(nonnull|align|dereferenceable)"));
> +        ASSERT_TRUE(MapHasRightValue(
> +            Map, {I->getOperand(0), Attribute::Dereferenceable}, {4, 48}));
> +        ASSERT_TRUE(MapHasRightValue(Map, {I->getOperand(0), Attribute::Alignment},
> +                               {8, 64}));
>         }));
>     Tests.push_back(std::make_pair(
>         "call void @func_many(i32* align 8 %P1) cold\n", [](Instruction *I) {
> @@ -332,8 +335,8 @@ TEST(AssumeQueryAPI, fillMapFromAssume) {
>           RetainedKnowledgeMap Map;
>           fillMapFromAssume(*Assume, Map);
>   
> -        AssertFindExactlyAttributes(
> -            Map, nullptr, "(nounwind|norecurse|willreturn|cold)");
> +        ASSERT_TRUE(FindExactlyAttributes(
> +            Map, nullptr, "(nounwind|norecurse|willreturn|cold)"));
>           ShouldPreserveAllAttributes.setValue(false);
>         }));
>     Tests.push_back(
> @@ -341,7 +344,7 @@ TEST(AssumeQueryAPI, fillMapFromAssume) {
>           RetainedKnowledgeMap Map;
>           fillMapFromAssume(*cast<CallInst>(I), Map);
>   
> -        AssertFindExactlyAttributes(Map, nullptr, "");
> +        ASSERT_TRUE(FindExactlyAttributes(Map, nullptr, ""));
>           ASSERT_TRUE(Map.empty());
>         }));
>     Tests.push_back(std::make_pair(
> @@ -357,32 +360,30 @@ TEST(AssumeQueryAPI, fillMapFromAssume) {
>           RetainedKnowledgeMap Map;
>           fillMapFromAssume(*Assume, Map);
>   
> -        AssertFindExactlyAttributes(Map, I->getOperand(0),
> -                                    "(readnone|align|dereferenceable|noalias)");
> -        AssertFindExactlyAttributes(Map, I->getOperand(1),
> -                                    "(align|dereferenceable)");
> -        AssertFindExactlyAttributes(Map, I->getOperand(2),
> -                                       "(align|dereferenceable)");
> -        AssertFindExactlyAttributes(Map, I->getOperand(3),
> -                                       "(nonnull|align|dereferenceable)");
> -        AssertMapHasRightValue(Map, {I->getOperand(0), Attribute::Alignment},
> -                               {32, 32});
> -        AssertMapHasRightValue(
> -            Map, {I->getOperand(0), Attribute::Dereferenceable}, {48, 48});
> -        AssertMapHasRightValue(
> -            Map, {I->getOperand(0), Attribute::NoAlias}, {0, 0});
> -        AssertMapHasRightValue(
> -            Map, {I->getOperand(1), Attribute::Dereferenceable}, {28, 28});
> -        AssertMapHasRightValue(Map, {I->getOperand(1), Attribute::Alignment},
> -                               {8, 8});
> -        AssertMapHasRightValue(Map, {I->getOperand(2), Attribute::Alignment},
> -                               {64, 64});
> -        AssertMapHasRightValue(
> -            Map, {I->getOperand(2), Attribute::Dereferenceable}, {4, 4});
> -        AssertMapHasRightValue(Map, {I->getOperand(3), Attribute::Alignment},
> -                               {16, 16});
> -        AssertMapHasRightValue(
> -            Map, {I->getOperand(3), Attribute::Dereferenceable}, {12, 12});
> +        ASSERT_TRUE(FindExactlyAttributes(Map, I->getOperand(0),
> +                                    "(align|dereferenceable)"));
> +        ASSERT_TRUE(FindExactlyAttributes(Map, I->getOperand(1),
> +                                    "(align|dereferenceable)"));
> +        ASSERT_TRUE(FindExactlyAttributes(Map, I->getOperand(2),
> +                                       "(align|dereferenceable)"));
> +        ASSERT_TRUE(FindExactlyAttributes(Map, I->getOperand(3),
> +                                       "(nonnull|align|dereferenceable)"));
> +        ASSERT_TRUE(MapHasRightValue(Map, {I->getOperand(0), Attribute::Alignment},
> +                               {32, 32}));
> +        ASSERT_TRUE(MapHasRightValue(
> +            Map, {I->getOperand(0), Attribute::Dereferenceable}, {48, 48}));
> +        ASSERT_TRUE(MapHasRightValue(
> +            Map, {I->getOperand(1), Attribute::Dereferenceable}, {28, 28}));
> +        ASSERT_TRUE(MapHasRightValue(Map, {I->getOperand(1), Attribute::Alignment},
> +                               {8, 8}));
> +        ASSERT_TRUE(MapHasRightValue(Map, {I->getOperand(2), Attribute::Alignment},
> +                               {64, 64}));
> +        ASSERT_TRUE(MapHasRightValue(
> +            Map, {I->getOperand(2), Attribute::Dereferenceable}, {4, 4}));
> +        ASSERT_TRUE(MapHasRightValue(Map, {I->getOperand(3), Attribute::Alignment},
> +                               {16, 16}));
> +        ASSERT_TRUE(MapHasRightValue(
> +            Map, {I->getOperand(3), Attribute::Dereferenceable}, {12, 12}));
>         }));
>   
>     /// Keep this test last as it modifies the function.
> @@ -398,15 +399,15 @@ TEST(AssumeQueryAPI, fillMapFromAssume) {
>   
>           Value *New = I->getFunction()->getArg(3);
>           Value *Old = I->getOperand(0);
> -        AssertFindExactlyAttributes(Map, New, "");
> -        AssertFindExactlyAttributes(Map, Old,
> -                                       "(nonnull|align|dereferenceable)");
> +        ASSERT_TRUE(FindExactlyAttributes(Map, New, ""));
> +        ASSERT_TRUE(FindExactlyAttributes(Map, Old,
> +                                       "(nonnull|align|dereferenceable)"));
>           Old->replaceAllUsesWith(New);
>           Map.clear();
>           fillMapFromAssume(*Assume, Map);
> -        AssertFindExactlyAttributes(Map, New,
> -                                       "(nonnull|align|dereferenceable)");
> -        AssertFindExactlyAttributes(Map, Old, "");
> +        ASSERT_TRUE(FindExactlyAttributes(Map, New,
> +                                       "(nonnull|align|dereferenceable)"));
> +        ASSERT_TRUE(FindExactlyAttributes(Map, Old, ""));
>         }));
>     RunTest(Head, Tail, Tests);
>   }
>
>
>          
> _______________________________________________
> 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