[llvm-commits] [llvm] r132732 - /llvm/trunk/utils/TableGen/FastISelEmitter.cpp

[David Blaikie]

>
> Just wondering about coding conventions - would it be more appropriate to
> change this from:
>   else if (cond) { ... } else { /* unreachable */ }
> to:
>   else { assert(cond); ... };
> This would be marginally more efficient in release builds, potentially.
>
>
> You're correct in pointing out that most sanity checks should be written
> with asserts (e.g., assert (cond && "msg")).  However, in the case of
> unreachable code we know that doom and gloom are sure to follow and thus we
> should gracefully abort and notify the user accordingly.  There's no
> performance to be gained because if we've reached this state the game is
> already over.
>
> In general, if you see something like assert(0 && "I should have never
> gotten here.."); it should be replaced with llvm_unreachable("I should
> have never gotten here..").  Notice that the condition is hardcoded to
> zero and thus the assert will always fire.
>

Right - but that's my point, there's performance to be gained by not at all
checking 'cond' in retail builds - whereas the code as you have it now will
check it unnecessarily & then go to llvm_unreachable if it's false. The perf
gain would be that retail builds would never check the condition because it
should never be false. That sounds like an assert to me, or am I missing
something?

Perhaps my example was oversimplified/unclear, what I meant was from this:

      } else if (Operands[i].isFP()) {
        OS << "ConstantFP *f" << i;
      } else {
        llvm_unreachable("Unknown operand kind!");
      }

to this:

      } else {
        assert(Operands[i].isFP() && "Unknown operator kind!");
        OS << "ConstantFP *f" << i;
      }
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110610/74859fa2/attachment.html>