On 09/27/2018 09:30 AM, Aleksey Shipilev wrote: > On 09/27/2018 10:21 AM, Andrew Haley wrote: >> On 09/27/2018 08:22 AM, Aleksey Shipilev wrote: >>> Otherwise looks good and trivial. >> >> No, it is neither good nor trivial:some of those should be >> ShouldNotReachHere(). >> I'll post a webrev later. > > Okay, let's see it. > > I was mostly concerned with having the same control flow as before, > did I miss some change that is actually non-trivial?
That's not my point: we should not put break statements in places we don't expect to reach. It's misleading for the reader. Code changes to "shut up" the compiler have do be done with great care and knowledge of what the code does. > On the second read, this change in c1_LIRAssembler_aarch64.cpp looks > suspicious, as it elevates ShouldNotReachHere to default case, > rather than letting default thing fall-through? > > break; > + default: > ShouldNotReachHere(); > } I think that one is actually OK. http://cr.openjdk.java.net/~aph/8211207/ follows the pattern of the x86 port. -- Andrew Haley Java Platform Lead Engineer Red Hat UK Ltd. <https://www.redhat.com> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
