This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by mkuper on Dec 22 2014, 7:06 AM.

Details

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.

Diff Detail

Event Timeline

mkuper updated this revision to Diff 17560.Dec 22 2014, 7:06 AM
mkuper retitled this revision from to Move recursive GlobalAlias handling to be after the max depth check in computeKnownBits().
mkuper updated this object.
mkuper edited the test plan for this revision. (Show Details)
mkuper added reviewers: chandlerc, majnemer.
mkuper added a subscriber: Unknown Object (MLST).
majnemer accepted this revision.Dec 22 2014, 10:45 AM
majnemer edited edge metadata.

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
}
This revision is now accepted and ready to land.Dec 22 2014, 10:45 AM

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

Thanks David, especially for the test-case!

This revision was automatically updated to reflect the committed changes.