RFR: 8183149: [AOT] SEGV in AMD64MathStub.pow: alignment for ArrayDataPointerConstant is not honored

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RFR: 8183149: [AOT] SEGV in AMD64MathStub.pow: alignment for ArrayDataPointerConstant is not honored

Bob Vandette
Please review these fixes to correct the alignment of the generated AOT library sections.
Although my initial implementation was compliant with the shared library specifications,
hotspot requires a greater alignment size.  These changes use the binary container
code segment size for setting alignment.

I also fixed a typo in the Elf createByteSection under this issue.

http://cr.openjdk.java.net/~bobv/8183149/webrev/

Bob.


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8183149: [AOT] SEGV in AMD64MathStub.pow: alignment for ArrayDataPointerConstant is not honored

Vladimir Kozlov
Thank you, Bob

Looks good.

Was it typo (3) in MachOSection.java?

-         // For now use 8 byte alignment
-         section.putInt(section_64.align.off, 3);
+         section.putInt(section_64.align.off, align);

Thanks,
Vladimir

On 7/25/17 12:22 PM, Bob Vandette wrote:

> Please review these fixes to correct the alignment of the generated AOT library sections.
> Although my initial implementation was compliant with the shared library specifications,
> hotspot requires a greater alignment size.  These changes use the binary container
> code segment size for setting alignment.
>
> I also fixed a typo in the Elf createByteSection under this issue.
>
> http://cr.openjdk.java.net/~bobv/8183149/webrev/
>
> Bob.
>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8183149: [AOT] SEGV in AMD64MathStub.pow: alignment for ArrayDataPointerConstant is not honored

Bob Vandette
I updated MachOSection.java to use log2 of align.

 
-        // For now use 8 byte alignment
-        section.putInt(section_64.align.off, 3);
+       section.putInt(section_64.align.off, (int)(Math.log10((double)align) / Math.log10((double)2.0));

Bob.

 

> On Jul 25, 2017, at 5:12 PM, Vladimir Kozlov <[hidden email]> wrote:
>
> Thank you, Bob
>
> Looks good.
>
> Was it typo (3) in MachOSection.java?
>
> -         // For now use 8 byte alignment
> -         section.putInt(section_64.align.off, 3);
> +         section.putInt(section_64.align.off, align);
>
> Thanks,
> Vladimir
>
> On 7/25/17 12:22 PM, Bob Vandette wrote:
>> Please review these fixes to correct the alignment of the generated AOT library sections.
>> Although my initial implementation was compliant with the shared library specifications,
>> hotspot requires a greater alignment size.  These changes use the binary container
>> code segment size for setting alignment.
>> I also fixed a typo in the Elf createByteSection under this issue.
>> http://cr.openjdk.java.net/~bobv/8183149/webrev/
>> Bob.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8183149: [AOT] SEGV in AMD64MathStub.pow: alignment for ArrayDataPointerConstant is not honored

Vladimir Kozlov
Why not?

31 - Integer.numberOfLeadingZeros(align);

Vladimir

On 7/27/17 11:02 AM, Bob Vandette wrote:

> I updated MachOSection.java to use log2 of align.
>
>  
> -        // For now use 8 byte alignment
> -        section.putInt(section_64.align.off, 3);
> +       section.putInt(section_64.align.off, (int)(Math.log10((double)align) / Math.log10((double)2.0));
>
> Bob.
>
>  
>> On Jul 25, 2017, at 5:12 PM, Vladimir Kozlov <[hidden email]> wrote:
>>
>> Thank you, Bob
>>
>> Looks good.
>>
>> Was it typo (3) in MachOSection.java?
>>
>> -         // For now use 8 byte alignment
>> -         section.putInt(section_64.align.off, 3);
>> +         section.putInt(section_64.align.off, align);
>>
>> Thanks,
>> Vladimir
>>
>> On 7/25/17 12:22 PM, Bob Vandette wrote:
>>> Please review these fixes to correct the alignment of the generated AOT library sections.
>>> Although my initial implementation was compliant with the shared library specifications,
>>> hotspot requires a greater alignment size.  These changes use the binary container
>>> code segment size for setting alignment.
>>> I also fixed a typo in the Elf createByteSection under this issue.
>>> http://cr.openjdk.java.net/~bobv/8183149/webrev/
>>> Bob.
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8183149: [AOT] SEGV in AMD64MathStub.pow: alignment for ArrayDataPointerConstant is not honored

Bob Vandette
I guess we can assume that we’ll get reasonable power of two values.
Yours is faster so I’ll make the change.

Thanks,
Bob.

> On Jul 27, 2017, at 2:09 PM, Vladimir Kozlov <[hidden email]> wrote:
>
> Why not?
>
> 31 - Integer.numberOfLeadingZeros(align);
>
> Vladimir
>
> On 7/27/17 11:02 AM, Bob Vandette wrote:
>> I updated MachOSection.java to use log2 of align.
>>  -        // For now use 8 byte alignment
>> -        section.putInt(section_64.align.off, 3);
>> +       section.putInt(section_64.align.off, (int)(Math.log10((double)align) / Math.log10((double)2.0));
>> Bob.
>>  
>>> On Jul 25, 2017, at 5:12 PM, Vladimir Kozlov <[hidden email]> wrote:
>>>
>>> Thank you, Bob
>>>
>>> Looks good.
>>>
>>> Was it typo (3) in MachOSection.java?
>>>
>>> -         // For now use 8 byte alignment
>>> -         section.putInt(section_64.align.off, 3);
>>> +         section.putInt(section_64.align.off, align);
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 7/25/17 12:22 PM, Bob Vandette wrote:
>>>> Please review these fixes to correct the alignment of the generated AOT library sections.
>>>> Although my initial implementation was compliant with the shared library specifications,
>>>> hotspot requires a greater alignment size.  These changes use the binary container
>>>> code segment size for setting alignment.
>>>> I also fixed a typo in the Elf createByteSection under this issue.
>>>> http://cr.openjdk.java.net/~bobv/8183149/webrev/
>>>> Bob.

Loading...