[PATCH] Move recursive GlobalAlias handling to be after the max depth check in computeKnownBits()
Eric Christopher
echristo at gmail.com
Mon Dec 22 11:10:55 PST 2014
One quick request. If ordering is important please comment it as such.
On Mon, Dec 22, 2014, 10:51 AM David Majnemer <david.majnemer at gmail.com>
wrote:
> LGTM with tweaks.
>
>
> ================
> Comment at: lib/Analysis/ValueTracking.cpp:797
> @@ -807,3 +796,3 @@
> // 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();
> ----------------
> You could make this more concise with `auto *GO =
> dyn_cast<GlobalObject>(V))`.
>
> ================
> Comment at: lib/Analysis/ValueTracking.cpp:800
> @@ -810,3 +799,3 @@
> if (Align == 0 && TD) {
> - if (GlobalVariable *GVar = dyn_cast<GlobalVariable>(GV)) {
> + if (GlobalVariable *GVar = dyn_cast<GlobalVariable>(GO)) {
> Type *ObjectType = GVar->getType()->getElementType();
> ----------------
> Might as well make this `auto *GV = dyn_cast<GlobalVariable>(GO))` now.
>
> ================
> Comment at: lib/Analysis/ValueTracking.cpp:850-851
> @@ +849,4 @@
> + if (GlobalAlias *GA = dyn_cast<GlobalAlias>(V)) {
> + if (GA->mayBeOverridden()) {
> + KnownZero.clearAllBits(); KnownOne.clearAllBits();
> + } else {
> ----------------
> 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.
>
> ================
> Comment at: test/Transforms/InstCombine/alias-recursion.ll:3-43
> @@ +2,43 @@
> +
> +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 (...)** }
> +
> + at 0 = private unnamed_addr constant [2 x i8*] [i8* bitcast
> (%rtti.CompleteObjectLocator* @"\01??_R4A@@6B@" to i8*), i8* bitcast (i32
> (%class.A*)* @"\01?virt at 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" }
> + at __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 at EA@A@@8"
> to i64), i64 ptrtoint (i8* @__ImageBase to i64)) to i32), i32 0]
> +@"\01??_R1A@?0A at 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 at A@@UEAAHXZ"(%class.A*) unnamed_addr align 2
> +
> ----------------
> 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
> }
> ```
>
> http://reviews.llvm.org/D6758
>
> EMAIL PREFERENCES
> http://reviews.llvm.org/settings/panel/emailpreferences/
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141222/7f048198/attachment.html>
More information about the llvm-commits
mailing list