[llvm] r305481 - [BasicAA] Don't call isKnownNonEqual if we might be have gone through a PHINode.

Mikael Holmén via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 22 02:38:43 PDT 2017


Hi Craig,

I managed to get a crash with this commit with

llc -o - foo.x86.ll -combiner-global-alias-analysis

llc: ../include/llvm/ADT/APInt.h:1287: bool 
llvm::APInt::intersects(const llvm::APInt &) const: Assertion `BitWidth 
== RHS.BitWidth && "Bit widths must be the same"' failed.

See below.

On 06/15/2017 07:16 PM, Craig Topper via llvm-commits wrote:
> Author: ctopper
> Date: Thu Jun 15 12:16:56 2017
> New Revision: 305481
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=305481&view=rev
> Log:
> [BasicAA] Don't call isKnownNonEqual if we might be have gone through a PHINode.
> 
> This is a fix for the test case in PR32314.
> 
> Basic Alias Analysis can ask if two nodes are known non-equal after looking through a phi node to find a GEP. isAddOfNonZero saw an add of a constant from the same phi and said that its output couldn't be equal. But Basic Alias Analysis was really asking about the value from the previous loop iteration.
> 
> This patch at least makes that case not happen anymore, I'm not sure if there were still other ways this can fail. As was discussed in the bug, it looks like fixing BasicAA would be difficult so this patch seemed like a possible workaround
> 
> Differential Revision: https://reviews.llvm.org/D33136
> 
> Modified:
>      llvm/trunk/lib/Analysis/BasicAliasAnalysis.cpp
> 
> Modified: llvm/trunk/lib/Analysis/BasicAliasAnalysis.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/BasicAliasAnalysis.cpp?rev=305481&r1=305480&r2=305481&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/BasicAliasAnalysis.cpp (original)
> +++ llvm/trunk/lib/Analysis/BasicAliasAnalysis.cpp Thu Jun 15 12:16:56 2017
> @@ -1011,10 +1011,24 @@ static AliasResult aliasSameBasePointerG
>       // equal each other so we can exit early.
>       if (C1 && C2)
>         return NoAlias;
> -    if (isKnownNonEqual(GEP1->getOperand(GEP1->getNumOperands() - 1),
> -                        GEP2->getOperand(GEP2->getNumOperands() - 1),
> -                        DL))
> -      return NoAlias;
> +    {
> +      Value *GEP1LastIdx = GEP1->getOperand(GEP1->getNumOperands() - 1);
> +      Value *GEP2LastIdx = GEP2->getOperand(GEP2->getNumOperands() - 1);
> +      if (isa<PHINode>(GEP1LastIdx) || isa<PHINode>(GEP2LastIdx)) {
> +        // If one of the indices is a PHI node, be safe and only use
> +        // computeKnownBits so we don't make any assumptions about the
> +        // relationships between the two indices. This is important if we're
> +        // asking about values from different loop iterations. See PR32314.
> +        // TODO: We may be able to change the check so we only do this when
> +        // we definitely looked through a PHINode.
> +        KnownBits Known1 = computeKnownBits(GEP1LastIdx, DL);
> +        KnownBits Known2 = computeKnownBits(GEP2LastIdx, DL);
> +        if (Known1.Zero.intersects(Known2.One) ||

When we come here we have

GEP1:
   %scevgep = getelementptr [97 x i8], [97 x i8]* @c, i8 0, i8 %lsr.iv

GEP2:
   %_tmp564 = getelementptr [97 x i8], [97 x i8]* @c, i8 0, i64 8

so Known1 and Known2 get different BitWidth and APInt::intersects hits 
the assert.

Regards,
Mikael

> +            Known1.One.intersects(Known2.Zero))
> +          return NoAlias;
> +      } else if (isKnownNonEqual(GEP1LastIdx, GEP2LastIdx, DL))
> +        return NoAlias;
> +    }
>       return MayAlias;
>     } else if (!LastIndexedStruct || !C1 || !C2) {
>       return MayAlias;
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> 
-------------- next part --------------
@c = external global [97 x i8]

define void @loops_test() {
bb19:
  br label %bb68

bb68:                                             ; preds = %bb71, %bb19
  %lsr.iv = phi i8 [ %lsr.iv.next, %bb71 ], [ 0, %bb19 ]
  %0 = add i8 %lsr.iv, 1
  %scevgep = getelementptr [97 x i8], [97 x i8]* @c, i8 0, i8 %lsr.iv
  %_tmp564 = getelementptr [97 x i8], [97 x i8]* @c, i8 0, i64 8
  store i8 undef, i8* %_tmp564
  %_tmp572 = load i8, i8* %scevgep
  %_tmp575 = icmp ne i8 %_tmp572, undef
  br i1 %_tmp575, label %bb70, label %bb71

bb70:                                             ; preds = %bb68
  unreachable

bb71:                                             ; preds = %bb68
  %lsr.iv.next = add i8 %lsr.iv, 1
  br label %bb68
}


More information about the llvm-commits mailing list