[9] RFR(S): 8049043: Load variable through a pointer of an incompatible type in hotspot/src/share/vm/runtime/sharedRuntimeMath.hpp

[Vladimir Kozlov]

David, I agree with your suggestion to pass union to _HI and _LO. The 
creation of a local union var makes sense then.

Thanks,
Vladimir

On 7/23/14 12:30 PM, David Chase wrote:
> Vladimir, I think the mistake is instead to include the cast to the type
> that it already is.
> I think performing the cast to the union type from a non-union address
> might get you a warning otherwise.
> I can confirm that if you write it plainly with the assignment to a
> correctly typed “conv”
> that clang (at least) generates good code:
>
> union ca {
>     double d;
>     int i[2];
> };
>
> int la(double d) {
>    union ca x = { d } ;
>    return x.i[0];
> }
>
> clang produces (-O3 -S, cleaned up)
>
> _la:                                    ## @la
>          pushq   %rbp
>          movq    %rsp, %rbp
>          movd    %xmm0, %rax
>          popq    %rbp
>          ret
> gcc 4.8.1 produces (-O4 -S, cleaned up)
>
> la:
> movsd%xmm0, -8(%rsp)
> movq-8(%rsp), %rax
> ret
>
> So I think this is generally a good idea, but the macro should be
> cleaned up to
> remove the cast that is certainly unnecessary now.  E.g.
>
>    39 #ifdef VM_LITTLE_ENDIAN
>    40 # define __HI(x) ((x).i[1])
>    41 # define __LO(x) ((x).i[0])
>    42 #else
>    43 # define __HI(x) (x.i[0])
>    44 # define __LO(x) (x.i[1])
>    45 #endif
>
> The temporary is only annoying to humans using a last-millenium dialect
> of C.  The code runs fast, and the address-of+dereference in the macro
> forced it to memory (lacking a lot of alias analysis) anyhow.
>
> David
>
>
> On Jul 23, 2014, at 1:58 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com
> <mailto:vladimir.kozlov at oracle.com>> wrote:
>
>> Tobias, why you are creating local copy 'conv' of a variable in all
>> places?
>>
>> Vladimir
>>
>> On 7/23/14 4:12 AM, Tobias Hartmann wrote:
>>> Hi,
>>>
>>> please review the following patch for JDK- 8049043 (Parfait warning).
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8049043
>>> Webrev: http://cr.openjdk.java.net/~thartmann/8049043/webrev.01/
>>>
>>> Problem:
>>> Parfait issues 6 warnings " Load variable through a pointer of an
>>> incompatible type" because of the __HI(x) and __LO(x) macros used in
>>> copysignA and scalbnA (share/vm/runtime/sharedRuntimeMath.hpp) to cast
>>> between double* and int*.
>>>
>>> Solution:
>>> A union 'DoubleIntConv' is used to access the high and low 32 bits of a
>>> double.
>>> The '# pragma warning( disable: 4748 )' in sharedRuntimeTrans.cpp is
>>> necessary because otherwise the VS compiler fails with the following
>>> warning: "/GS can not protect parameters and local variables from local
>>> buffer overrun because optimizations are disabled in function". There
>>> exists a bug to fix this [1].
>>>
>>> Testing:
>>> JPRT, Parfait (all 6 warnings removed)
>>>
>>> Thanks,
>>> Tobias
>>>
>>> [1] https://bugs.openjdk.java.net/browse/JDK-8046696
>