[PATCH] D52002: Switch optimization for known maximum switch values

Tue Dec 18 07:21:50 PST 2018

hans added a comment.

In D52002#1334371 <https://reviews.llvm.org/D52002#1334371>, @ayonam wrote:

> In D52002#1334222 <https://reviews.llvm.org/D52002#1334222>, @hans wrote:
>
> > If you can provide more details about what didn't work, maybe I can help investigate. (Though I'm about to go on holiday soon, so probably not until January.)
>
>
> There are three files that when compiled with this patch, generate wrong code, viz., AArch64LoadStoreOptimizer.cpp, AArch64InstrInfo.cpp and AArch64ConditionalCompares.cpp.  Out of these we tried to isolate the problem with the last one.  I figured out that if the functions SSACCmpConv::findConvertibleCompare() and SSACCmpConv::convert() are compiled without this patch, the code works fine.  So the problem surfaces with these two routines only.  There are a few switch cases in those two routines but I couldn't see anything exceptional with those except for a call to __builtin_unreachable() in the default case for two of the switches and a [[clang::fallthrough]] in another.  In all these three cases, I was unable to figure out how they could possibly break our assumptions.  Does the __builtin_unreachable() have any special semantic that we are not handling?

Does the error show with the regular lit tests, or do you have some internal test that fails?

My first guess would be that one of the "unreachable" defaults aren't actually unreachable for some input. But then they should trap in an asserts-enabled build..

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D52002/new/

https://reviews.llvm.org/D52002