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
Hi, Here's something I don't understand... How come that UNDEF can appear as a branch condition at all? I just can't think of any ways. If you write something like fun() { int x; if (x > 100) { ... } else { ... } } LLVM generates a boolean temporary that compares (uninitialized) value of x with 100. Second, if it already can appear, isn't that a bug that should be reported by the compiler? Domagoj -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20060607/eac27aea/attachment.html>
Domagoj Babic wrote:> Here's something I don't understand... How come that UNDEF can > appear as a branch condition at all? I just can't think of any ways. > > If you write something like > > fun() { > int x; > if (x > 100) { > ... > } else { > ... > } > } > > LLVM generates a boolean temporary that compares (uninitialized) > value of x with 100.Firstly, "int x" generates an alloca of one int. The use of "x > 100" then generates a load of that int. The instruction combiner detects that it's an alloca+load with no intervening store, and replaces it with undef. Then, the instruction combiner will try to constant fold the setgt operation with undef, 100 as arguments. That folds into undef. So before SCCP is run, your example does produce "if (undef)".> Second, if it already can appear, isn't that a bug that should be > reported by the compiler?Almost. If that undef never executes, then there is no bug, but that depends on run-time conditions. I suppose you could warn about it anyways, or perhaps even replace such BBs with "unreachable" though that might alarm too many users. The right place to check for these sorts of things might be at link/JIT time if undef usage can't be optimized away as dead code. I'm not sure users are ready for their linker to be emitting those sorts of errors though. Nick
On Wed, 7 Jun 2006, Domagoj Babic wrote:> Here's something I don't understand... How come that UNDEF can > appear as a branch condition at all? I just can't think of any ways.Lots of different ways, e.g.: fun() { _Bool C; if (C) { ... } }> Second, if it already can appear, isn't that a bug that should be > reported by the compiler?That's up to the front-end. If you turn on enough warnings you should get one. At the compiler level, they can occur due to unrealizable code paths (not necessarily bugs in the user code), so it's useful for the optimizer to take advantage of it. -Chris -- http://nondot.org/sabre/ http://llvm.org/