I found that "undef" was disappearing early into the optimization chain. SCCP was the culprit, transforming: br bool undef, label %T, label %F into br bool true, label %T, label %F While that sounds like a great optimization, it shouldn't be happening that early. I've put together a patch which modifies the behaviour of SCCP so that it preserves undef in those cases. I had to modify one of the regression tests which was trying to test IPSCCP on global variables, but was uses undef and expecting the above transformation to take place. I changed its global from undef to true, and wrote a new testcase for the undef-related part. Patch attached. Comments? Nick Lewycky -------------- next part -------------- A non-text attachment was scrubbed... Name: sccp-branch-undef.patch Type: text/x-patch Size: 3800 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20060605/1da45df2/attachment.bin>
Nick Lewycky wrote:> I found that "undef" was disappearing early into the optimization chain. > SCCP was the culprit, transforming: > > br bool undef, label %T, label %F > > into > > br bool true, label %T, label %F > > While that sounds like a great optimization, it shouldn't be happening > that early.Uh, why?
Daniel Berlin wrote:> Nick Lewycky wrote: > >>I found that "undef" was disappearing early into the optimization chain. >>SCCP was the culprit, transforming: >> >> br bool undef, label %T, label %F >> >>into >> >> br bool true, label %T, label %F >> >>While that sounds like a great optimization, it shouldn't be happening >>that early. > > Uh, why?It might not always be optimal to replace undef with true at that point. If the true branch does a lot of work, and the false branch is empty, and the program is valid either way, you'd rather choose the false branch. I admit that it's minor (undef isn't supposed to happen a whole lot anyways), but we should be deciding this sort of thing at link time where the behaviour of the two branches is guaranteed to be visible. For some reason I thought that ADCE already did that sort of resolution on 'br bool undef', but I seem to have mistested. Before anyone applies this patch, you should probably wait for a patch to the ADCE so that it removes 'br undef'. In practise, it'll just replace undef with true, so until someone writes a better replacement, we'll end up par anyways. Nick