[PATCH] D27028: Add intrinsics for constrained floating point operations

[Andy Kaylor via Phabricator via llvm-commits]

andrew.w.kaylor added a comment.

In https://reviews.llvm.org/D27028#613949, @mehdi_amini wrote:

> In https://reviews.llvm.org/D27028#613926, @andrew.w.kaylor wrote:
>
> > In https://reviews.llvm.org/D27028#613867, @mehdi_amini wrote:
> >
> > > Having the llvm_unreachable right after the StringSwitch should achieve the same thing.
> >
> >
> > I'm not sure I understand what you're suggesting here.  If I do this:
> >
> >   return StringSwitch<RoundingMode>(RoundingArg)
> >     .Case("round.dynamic",    rmDynamic)
> >     .Case("round.tonearest",  rmToNearest)
> >     .Case("round.downward",   rmDownward)
> >     .Case("round.upward",     rmUpward)
> >     .Case("round.towardzero", rmTowardZero);
> >   llvm_unreachable("Unexpected rounding mode argument in FP intrinsic!");
> >   
> >
> > the llvm_unreachable statement is purely unreachable because the implicit default from StringSwitch will assert or dereference a null pointer.  The llvm_unreachable in this case effectively becomes a comment.
>
>
> Not really: in an optimized build it means that the default case is unreachable. The assertion does not exist there, and the optimizer can drop the nullptr dereference.


But the 'return' will always return, won't it?  I don't see how the optimizer could associate the llvm_unreachable statement with the 'return StringSwitch(...)' to be able to do anything with it.

I'm in the process of preparing an updated revision that implements UnreachableDefault as I suggested, but I'm not overly attached to that approach.  If you feel strongly that something else would be better I can change it.


Repository:
  rL LLVM

https://reviews.llvm.org/D27028