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

[David Chase]

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> 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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20140723/6ff4bf56/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 842 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20140723/6ff4bf56/signature-0001.asc>