Move recursive GlobalAlias handling to be after the max depth check in computeKnownBits()

[mkuper]

Summary

r220165 moves GlobalAlias handling to be before GlobalValue handling, otherwise it's dead code. Unfortunately, that also moved it to be before the max depth check, causing an assert.
Moved it forward to where it's safe, and changed the GlobalValue handling to only look at GlobalObjects.

Sorry for the awful test-case, this is bugpoint-reduced C++ with a bit of manual clean-up. I can try to clean it up more, if necessary.

[majnemer]

LGTM with tweaks.

lib/Analysis/ValueTracking.cpp

797 ↗

(On Diff #17560)

You could make this more concise with auto *GO = dyn_cast<GlobalObject>(V)).

800 ↗

(On Diff #17560)

Might as well make this auto *GV = dyn_cast<GlobalVariable>(GO)) now.

850–851 ↗

(On Diff #17560)

We just cleared out all the bits on line 842, we probably don't need to do it again. Instead, I'd replace it with an assert that KnownZero and KnownOne are both zero.

test/Transforms/InstCombine/alias-recursion.ll

3–43 ↗

(On Diff #17560)

I believe the following is more minimal:

target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-pc-windows-msvc"

%class.A = type { i32 (...)** }

@0 = constant [1 x i8*] zeroinitializer

@vtbl = alias getelementptr inbounds ([1 x i8*]* @0, i32 0, i32 0)

define i32 (%class.A*)* @test() {
entry:
  br i1 undef, label %for.body, label %for.end

for.body:                                         ; preds = %for.body, %entry
  br i1 undef, label %for.body, label %for.end

for.end:                                          ; preds = %for.body, %entry
  %A = phi i32 (%class.A*)** [ bitcast (i8** @vtbl to i32 (%class.A*)**), %for.body ], [ null, %entry ]
  %B = load i32 (%class.A*)** %A
  ret i32 (%class.A*)* %B
}

[echristo]

One quick request. If ordering is important please comment it as such.

[mkuper]

Thanks David, especially for the test-case!

llvm/trunk/lib/Analysis/ValueTracking.cpp

if (ConstantDataSequential *CDS = dyn_cast<ConstantDataSequential>(V)) {
     for (unsigned i = 0, e = CDS->getNumElements(); i != e; ++i) {
       Elt = CDS->getElementAsInteger(i);
       KnownZero &= ~Elt;
       KnownOne &= Elt;
     }
     return;
   }
 
-  // A weak GlobalAlias is totally unknown. A non-weak GlobalAlias has
-  // the bits of its aliasee.
-  if (GlobalAlias *GA = dyn_cast<GlobalAlias>(V)) {
-    if (GA->mayBeOverridden()) {
-      KnownZero.clearAllBits(); KnownOne.clearAllBits();
-    } else {
-      computeKnownBits(GA->getAliasee(), KnownZero, KnownOne, TD, Depth+1, Q);
-    }
-    return;
-  }
-
   // The address of an aligned GlobalValue has trailing zeros.
-  if (GlobalValue *GV = dyn_cast<GlobalValue>(V)) {
-    unsigned Align = GV->getAlignment();
+  if (auto *GO = dyn_cast<GlobalObject>(V)) {
+    unsigned Align = GO->getAlignment();
     if (Align == 0 && TD) {
-      if (GlobalVariable *GVar = dyn_cast<GlobalVariable>(GV)) {
+      if (auto *GVar = dyn_cast<GlobalVariable>(GO)) {
         Type *ObjectType = GVar->getType()->getElementType();
         if (ObjectType->isSized()) {
           // If the object is defined in the current Module, we'll be giving
           // it the preferred alignment. Otherwise, we have to assume that it
           // may only have the minimum ABI alignment.
           if (!GVar->isDeclaration() && !GVar->isWeakForLinker())
             Align = TD->getPreferredAlignment(GVar);
           else
if (Argument *A = dyn_cast<Argument>(V)) {
     // information...
     computeKnownBitsFromAssume(V, KnownZero, KnownOne, TD, Depth, Q);
     return;
   }
 
   // Start out not knowing anything.
   KnownZero.clearAllBits(); KnownOne.clearAllBits();
 
+  // Limit search depth.
+  // All recursive calls that increase depth must come after this.
   if (Depth == MaxDepth)
-    return;  // Limit search depth.
+    return;  
+
+  // A weak GlobalAlias is totally unknown. A non-weak GlobalAlias has
+  // the bits of its aliasee.
+  if (GlobalAlias *GA = dyn_cast<GlobalAlias>(V)) {
+    if (!GA->mayBeOverridden())
+      computeKnownBits(GA->getAliasee(), KnownZero, KnownOne, TD, Depth + 1, Q);
+    return;
+  }
 
   // Check whether a nearby assume intrinsic can determine some known bits.
   computeKnownBitsFromAssume(V, KnownZero, KnownOne, TD, Depth, Q);
 
   Operator *I = dyn_cast<Operator>(V);
   if (!I) return;
 
   APInt KnownZero2(KnownZero), KnownOne2(KnownOne);

llvm/trunk/test/Transforms/InstCombine/alias-recursion.ll

+; RUN: opt < %s -instcombine -S | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc"
+
+%class.A = type { i32 (...)** }
+
+@0 = constant [1 x i8*] zeroinitializer
+
+@vtbl = alias getelementptr inbounds ([1 x i8*]* @0, i32 0, i32 0)
+
+define i32 (%class.A*)* @test() {
+; CHECK-LABEL: test
+entry:
+  br i1 undef, label %for.body, label %for.end
+
+for.body:                                         ; preds = %for.body, %entry
+  br i1 undef, label %for.body, label %for.end
+
+for.end:                                          ; preds = %for.body, %entry
+  %A = phi i32 (%class.A*)** [ bitcast (i8** @vtbl to i32 (%class.A*)**), %for.body ], [ null, %entry ]
+  %B = load i32 (%class.A*)** %A
+  ret i32 (%class.A*)* %B
+}