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

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

800

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

850–851

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

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!

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 (GlobalObject *GO = dyn_cast<GlobalObject>(V)) {
+    unsigned Align = GO->getAlignment();
     if (Align == 0 && TD) {
-      if (GlobalVariable *GVar = dyn_cast<GlobalVariable>(GV)) {
+      if (GlobalVariable *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
void computeKnownBits(Value *V, APInt &KnownZero, APInt &KnownOne,
   }
 
   // Start out not knowing anything.
   KnownZero.clearAllBits(); KnownOne.clearAllBits();
 
   if (Depth == MaxDepth)
     return;  // Limit search depth.
 
+  // 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;
+  }
+
   // 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);
   switch (I->getOpcode()) {

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"
+
+%rtti.CompleteObjectLocator = type { i32, i32, i32, i32, i32, i32 }
+%rtti.TypeDescriptor = type { i8**, i8*, [8 x i8] }
+%rtti.ClassHierarchyDescriptor = type { i32, i32, i32, i32 }
+%rtti.BaseClassDescriptor = type { i32, i32, i32, i32, i32, i32, i32 }
+%class.A = type { i32 (...)** }
+
+@0 = private unnamed_addr constant [2 x i8*] [i8* bitcast (%rtti.CompleteObjectLocator* @"\01??_R4A@@6B@" to i8*), i8* bitcast (i32 (%class.A*)* @"\01?virt@A@@UEAAHXZ" to i8*)]
+@"\01??_R4A@@6B@" = linkonce_odr constant %rtti.CompleteObjectLocator { i32 1, i32 0, i32 0, i32 trunc (i64 sub nuw nsw (i64 ptrtoint (%rtti.TypeDescriptor* @"\01??_R0?AVA@@@8" to i64), i64 ptrtoint (i8* @__ImageBase to i64)) to i32), i32 trunc (i64 sub nuw nsw (i64 ptrtoint (%rtti.ClassHierarchyDescriptor* @"\01??_R3A@@8" to i64), i64 ptrtoint (i8* @__ImageBase to i64)) to i32), i32 trunc (i64 sub nuw nsw (i64 ptrtoint (%rtti.CompleteObjectLocator* @"\01??_R4A@@6B@" to i64), i64 ptrtoint (i8* @__ImageBase to i64)) to i32) }
+@"\01??_7type_info@@6B@" = external constant i8*
+@"\01??_R0?AVA@@@8" = linkonce_odr global %rtti.TypeDescriptor { i8** @"\01??_7type_info@@6B@", i8* null, [8 x i8] c".?AVA@@\00" }
+@__ImageBase = external constant i8
+@"\01??_R3A@@8" = linkonce_odr constant %rtti.ClassHierarchyDescriptor { i32 0, i32 0, i32 1, i32 trunc (i64 sub nuw nsw (i64 ptrtoint ([2 x i32]* @"\01??_R2A@@8" to i64), i64 ptrtoint (i8* @__ImageBase to i64)) to i32) }
+@"\01??_R2A@@8" = linkonce_odr constant [2 x i32] [i32 trunc (i64 sub nuw nsw (i64 ptrtoint (%rtti.BaseClassDescriptor* @"\01??_R1A@?0A@EA@A@@8" to i64), i64 ptrtoint (i8* @__ImageBase to i64)) to i32), i32 0]
+@"\01??_R1A@?0A@EA@A@@8" = linkonce_odr constant %rtti.BaseClassDescriptor { i32 trunc (i64 sub nuw nsw (i64 ptrtoint (%rtti.TypeDescriptor* @"\01??_R0?AVA@@@8" to i64), i64 ptrtoint (i8* @__ImageBase to i64)) to i32), i32 0, i32 0, i32 -1, i32 0, i32 64, i32 trunc (i64 sub nuw nsw (i64 ptrtoint (%rtti.ClassHierarchyDescriptor* @"\01??_R3A@@8" to i64), i64 ptrtoint (i8* @__ImageBase to i64)) to i32) }
+@"\01??_7A@@6B@" = unnamed_addr alias getelementptr inbounds ([2 x i8*]* @0, i32 0, i32 1)
+
+define void @test() {
+; CHECK-LABEL: test
+entry:
+  br i1 undef, label %for.body.lr.ph, label %for.end
+
+for.body.lr.ph:                                   ; preds = %entry
+  br label %for.body
+
+for.body:                                         ; preds = %for.body.for.body_crit_edge, %for.body.lr.ph
+  %vtable = phi i32 (%class.A*)** [ bitcast (i8** @"\01??_7A@@6B@" to i32 (%class.A*)**), %for.body.lr.ph ], [ null, %for.body.for.body_crit_edge ]
+  %0 = load i32 (%class.A*)** %vtable, align 8
+  %call2 = tail call i32 %0(%class.A* undef)
+  br i1 undef, label %for.body.for.body_crit_edge, label %for.end
+
+for.body.for.body_crit_edge:                      ; preds = %for.body
+  br label %for.body
+
+for.end:                                          ; preds = %for.body, %entry
+  ret void
+}
+
+declare i32 @"\01?virt@A@@UEAAHXZ"(%class.A*) unnamed_addr align 2
+
+