[cfe-commits] r140232 - /cfe/trunk/lib/Sema/SemaExpr.cpp

[David Blaikie]

On Wed, Sep 21, 2011 at 3:36 PM, Chandler Carruth <chandlerc at google.com>wrote:

> On Wed, Sep 21, 2011 at 3:25 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>> One other point on this - the issues you've raised already exist in both
>> assert and llvm_unreachable code, making this change (assert(0) ->
>> llvm_unreachable) somewhat orthogonal to them.
>>
>
> Not at all. llvm_unreachable has a tremendously different impact in
> optimized builds than assert(0) does. The latter has zero impact, the
> control flow remains as expected.
>
> But llvm_unreachable tells the optimizer that this code path *cannot be
> taken*. That in turn constrains the value of the input. That in turn causes
> other optimizations to fire, and so on. This can cause essentially unbounded
> undefined behavior given an unexpected input, where as the assert will
> "merely" fall through.
>
> I've specifically seen cases in the disassembler and assembler code where
> truly unexpected cases are assert(0)'ed, but then handled benignly.
>

That's pretty much pure luck & potentially just hides the bug for longer in
retail builds that might otherwise fail more catastrophically (or less,
certainly - but I don't think it's strictly better to have release builds be
agnostic to these paths)


> I don't think we want to tell the optimizer that this *cannot* happen, but
> rather catch when it does happen in our test suite. The assert seems well
> suited to that.
>

Is this true even in debug builds? My understanding was that
llvm_unreachable was much like an assert in debug builds - otherwise the use
of a message is rather useless since the compiler might not even have such
code anymore.

If it is true, should we consider changing our llvm_unreachable usage to
assert(0s? That's what I was getting at above. We're already treating the
same case in two different ways (some times with assert, sometimes with
llvm_unreachable) & it'd be nice to figure out the right way.

[on that note about optimizations - it'd be nice if our asserts could be
used to implement the same kind of cascading optimizations that
llvm_unreachable provides, wouldn't it?]

- David
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110921/695f28da/attachment.html>