RFR (XS) 8217321: [TESTBUG] utilities/test_globalDefinitions.cpp should use _LP64, not LP64

[Aleksey Shipilev]

On 1/18/19 12:42 AM, David Holmes wrote:
> On 18/01/2019 9:20 am, Aleksey Shipilev wrote:
>> On 1/17/19 11:12 PM, David Holmes wrote:
>>> When we build hotspot we set LP64 (whereas _LP64 presumably comes from the compiler). Shouldn't the
>>> gtests get compiled with the same flags as the product code?
>>
>> Yes, I think so, and that is what happens here? Everywhere else in Hotspot we use "#ifdef _LP64",
>> and this patch makes use of the same in gtests as well.
> 
> I've no issue with the change, but if the same flags are used then they tests should have been
> executed anyway. If they weren't then the same flags are not being used and perhaps they should?

I think the basic premise "LP64 is set" is incorrect. _LP64 is defined, not LP64, in both hotspot
and gtest compilation on Linux x86_64. I don't see the problem: flags are consistent between hotspot
and gtest, and gtests cannot just use LP64 on their own. Look:

$ hg diff -U 2
diff -r e1da82072c79 src/hotspot/share/opto/library_call.cpp
--- a/src/hotspot/share/opto/library_call.cpp   Fri Jan 18 09:04:09 2019 +0100
+++ b/src/hotspot/share/opto/library_call.cpp   Fri Jan 18 10:22:01 2019 +0100
@@ -341,4 +341,11 @@
 //---------------------------make_vm_intrinsic----------------------------
 CallGenerator* Compile::make_vm_intrinsic(ciMethod* m, bool is_virtual) {
+#ifdef LP64
+  Non-compilable cruft.
+#endif
+#ifndef _LP64
+  Non-compilable cruft.
+#endif
+
   vmIntrinsics::ID id = m->intrinsic_id();
   assert(id != vmIntrinsics::_none, "must be a VM intrinsic");
diff -r e1da82072c79 test/hotspot/gtest/utilities/test_globalDefinitions.cpp
--- a/test/hotspot/gtest/utilities/test_globalDefinitions.cpp   Fri Jan 18 09:04:09 2019 +0100
+++ b/test/hotspot/gtest/utilities/test_globalDefinitions.cpp   Fri Jan 18 10:22:01 2019 +0100
@@ -95,4 +95,11 @@

 TEST(globalDefinitions, exact_unit_for_byte_size) {
+#ifdef LP64
+  Non-compilable cruft.
+#endif
+#ifndef _LP64
+  Non-compilable cruft.
+#endif
+
   EXPECT_STREQ("B", exact_unit_for_byte_size(0));
   EXPECT_STREQ("B", exact_unit_for_byte_size(1));

$ CONF=linux-x86_64-server-fastdebug make clean images
<SUCCESS!>

$ CONF=linux-x86_64-server-fastdebug make clean run-test TEST=gtest:globalDefinitions
<SUCCESS!>

>> $ ack "ifdef LP64" src/hotspot/ | wc -l
>> 0
> 
> Try ifndef LP64 - there are a few of those in ./cpu/x86/x86.ad

Oh, .ad is compiled with ADLC, which is yet another build config. I think the use of "ifndef LP64"
there is incorrect, and it should use _LP64 as well. In fact, the rest of x86.ad uses _LP64, and
that LP64 looks like a recent addition. And that code even fails to compile when ifndef is fixed! See:
  https://bugs.openjdk.java.net/browse/JDK-8217371

-Aleksey