[aarch64-port-dev ] RFR(XS): 8211207: AArch64: Fix build failure after JDK-8211029

[Dmitry Samersoff]

Andrew,

My $0.2

It might be better to refactor
switch(tag) in c1_LIRGenerator_aarch64.cpp
to usual case/break form, rather than have multiple returns.

i.e.

switch (tag) {
 583     case floatTag: // fall trough
 584     case doubleTag:
           do_ArithmeticOp_FPU(x);
           break;
 585     case longTag:
           do_ArithmeticOp_Long(x);
           break;
 586     case intTag:
           do_ArithmeticOp_Int(x);
           break;
 587     default:
           ShouldNotReachHere();
 588   }

        return;

-Dmitry


On 27.09.2018 11:53, Andrew Haley wrote:
> 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.
> 


-- 
Dmitry Samersoff
http://devnull.samersoff.net
* There will come soft rains ...