RFR(S): 8183571: PPC64 build broken after 8178499

[Stefan Karlsson]

On 2017-07-05 16:15, Stefan Karlsson wrote:
> Hi Marti,
> 
> On 2017-07-05 15:55, Doerr, Martin wrote:
>> Hi,
>>
>> I have fixed the PPC64 build:
>> http://cr.openjdk.java.net/~mdoerr/8183571_fix_PPC64_build/webrev.00/
>> (Contains minor PPC code improvements.)
>>
>> Please review.
> 
> Most looks good to me, but I'm not sure why this was needed because of 
> 8178499:
> -  if (has_mfdscr()) {
> +  if (PowerArchitecturePPC64 >= 8 && has_mfdscr()) {
> 
> I'm also not sure why you needed this change:
> -  (*test)(align_up(mid_of_test_area, 16), (uint64_t)0);
> +  (*test)((address)align_up((intptr_t)mid_of_test_area, 16), 0);

And now I read the bug report and understand why need to cast here.

Another way to write this could probably be:
(*test)(align_up((address)mid_of_test_area, 16), 0);

Or maybe even stop mixing address and char*, but that might be much 
larger task.

StefanK

> 
> Did you consider changing:
>    enum {
>      // stack alignment
>      alignment_in_bytes = 16,
>      // log_2(16*8 bits) = 7.
>      log_2_of_alignment_in_bits = 7
>    };
> 
> to the more type safe:
>      // stack alignment
>      static const int alignment_in_bytes = 16;
>      // log_2(16*8 bits) = 7.
>      static const int log_2_of_alignment_in_bits = 7
> 
> that way you wouldn't have to cast in code like this:
> -  const int frame_size_in_bytes  = align_up(register_save_size, 
> frame::alignment_in_bytes)
> +  const int frame_size_in_bytes  = align_up(register_save_size, 
> (int)frame::alignment_in_bytes)
> 
> StefanK
> 
>>
>> Best regards,
>> Martin
>>