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

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added a comment.

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.

> So what I'd like to do is 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)
>     .UnreachableDefault("Unexpected rounding mode argument in FP intrinsic!");
>    
> 
> which at least in !NDEBUG builds will produce a useful message.  This would be fairly trivial to implement.

Right the only difference is that you get a nicer runtime failure in assert mode.


Repository:
  rL LLVM

https://reviews.llvm.org/D27028