<p dir="ltr">One quick request. If ordering is important please comment it as such. </p>
<br><div class="gmail_quote">On Mon, Dec 22, 2014, 10:51 AM David Majnemer <<a href="mailto:david.majnemer@gmail.com">david.majnemer@gmail.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">LGTM with tweaks.<br>
<br>
<br>
================<br>
Comment at: lib/Analysis/ValueTracking.<u></u>cpp:797<br>
@@ -807,3 +796,3 @@<br>
   // The address of an aligned GlobalValue has trailing zeros.<br>
-  if (GlobalValue *GV = dyn_cast<GlobalValue>(V)) {<br>
-    unsigned Align = GV->getAlignment();<br>
+  if (GlobalObject *GO = dyn_cast<GlobalObject>(V)) {<br>
+    unsigned Align = GO->getAlignment();<br>
----------------<br>
You could make this more concise with `auto *GO = dyn_cast<GlobalObject>(V))`.<br>
<br>
================<br>
Comment at: lib/Analysis/ValueTracking.<u></u>cpp:800<br>
@@ -810,3 +799,3 @@<br>
     if (Align == 0 && TD) {<br>
-      if (GlobalVariable *GVar = dyn_cast<GlobalVariable>(GV)) {<br>
+      if (GlobalVariable *GVar = dyn_cast<GlobalVariable>(GO)) {<br>
         Type *ObjectType = GVar->getType()-><u></u>getElementType();<br>
----------------<br>
Might as well make this `auto *GV = dyn_cast<GlobalVariable>(GO))` now.<br>
<br>
================<br>
Comment at: lib/Analysis/ValueTracking.<u></u>cpp:850-851<br>
@@ +849,4 @@<br>
+  if (GlobalAlias *GA = dyn_cast<GlobalAlias>(V)) {<br>
+    if (GA->mayBeOverridden()) {<br>
+      KnownZero.clearAllBits(); KnownOne.clearAllBits();<br>
+    } else {<br>
----------------<br>
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.<br>
<br>
================<br>
Comment at: test/Transforms/InstCombine/<u></u>alias-recursion.ll:3-43<br>
@@ +2,43 @@<br>
+<br>
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:<u></u>32:64-S128"<br>
+target triple = "x86_64-pc-windows-msvc"<br>
+<br>
+%rtti.CompleteObjectLocator = type { i32, i32, i32, i32, i32, i32 }<br>
+%rtti.TypeDescriptor = type { i8**, i8*, [8 x i8] }<br>
+%rtti.<u></u>ClassHierarchyDescriptor = type { i32, i32, i32, i32 }<br>
+%rtti.BaseClassDescriptor = type { i32, i32, i32, i32, i32, i32, i32 }<br>
+%class.A = type { i32 (...)** }<br>
+<br>
+@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*)]<br>
+@"\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.<u></u>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) }<br>
+@"\01??_7type_info@@6B@" = external constant i8*<br>
+@"\01??_R0?AVA@@@8" = linkonce_odr global %rtti.TypeDescriptor { i8** @"\01??_7type_info@@6B@", i8* null, [8 x i8] c".?AVA@@\00" }<br>
+@__ImageBase = external constant i8<br>
+@"\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) }<br>
+@"\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]<br>
+@"\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.<u></u>ClassHierarchyDescriptor* @"\01??_R3A@@8" to i64), i64 ptrtoint (i8* @__ImageBase to i64)) to i32) }<br>
+@"\01??_7A@@6B@" = unnamed_addr alias getelementptr inbounds ([2 x i8*]* @0, i32 0, i32 1)<br>
+<br>
+define void @test() {<br>
+; CHECK-LABEL: test<br>
+entry:<br>
+  br i1 undef, label %<a href="http://for.body.lr.ph" target="_blank">for.body.lr.ph</a>, label %for.end<br>
+<br>
+<a href="http://for.body.lr.ph" target="_blank">for.body.lr.ph</a>:                                   ; preds = %entry<br>
+  br label %for.body<br>
+<br>
+for.body:                                         ; preds = %for.body.for.body_crit_edge, %<a href="http://for.body.lr.ph" target="_blank">for.body.lr.ph</a><br>
+  %vtable = phi i32 (%class.A*)** [ bitcast (i8** @"\01??_7A@@6B@" to i32 (%class.A*)**), %<a href="http://for.body.lr.ph" target="_blank">for.body.lr.ph</a> ], [ null, %for.body.for.body_crit_edge ]<br>
+  %0 = load i32 (%class.A*)** %vtable, align 8<br>
+  %call2 = tail call i32 %0(%class.A* undef)<br>
+  br i1 undef, label %for.body.for.body_crit_edge, label %for.end<br>
+<br>
+for.body.for.body_crit_edge:                      ; preds = %for.body<br>
+  br label %for.body<br>
+<br>
+for.end:                                          ; preds = %for.body, %entry<br>
+  ret void<br>
+}<br>
+<br>
+declare i32 @"\01?virt@A@@UEAAHXZ"(%class.<u></u>A*) unnamed_addr align 2<br>
+<br>
----------------<br>
I believe the following is more minimal:<br>
<br>
```<br>
target datalayout = "e-m:e-i64:64-f80:128-n8:16:<u></u>32:64-S128"<br>
target triple = "x86_64-pc-windows-msvc"<br>
<br>
%class.A = type { i32 (...)** }<br>
<br>
@0 = constant [1 x i8*] zeroinitializer<br>
<br>
@vtbl = alias getelementptr inbounds ([1 x i8*]* @0, i32 0, i32 0)<br>
<br>
define i32 (%class.A*)* @test() {<br>
entry:<br>
  br i1 undef, label %for.body, label %for.end<br>
<br>
for.body:                                         ; preds = %for.body, %entry<br>
  br i1 undef, label %for.body, label %for.end<br>
<br>
for.end:                                          ; preds = %for.body, %entry<br>
  %A = phi i32 (%class.A*)** [ bitcast (i8** @vtbl to i32 (%class.A*)**), %for.body ], [ null, %entry ]<br>
  %B = load i32 (%class.A*)** %A<br>
  ret i32 (%class.A*)* %B<br>
}<br>
```<br>
<br>
<a href="http://reviews.llvm.org/D6758" target="_blank">http://reviews.llvm.org/D6758</a><br>
<br>
EMAIL PREFERENCES<br>
  <a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/<u></u>settings/panel/<u></u>emailpreferences/</a><br>
<br>
<br>
<br>
______________________________<u></u>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
</blockquote></div>