RFR: 8203030: Zero s390 31 bit size_t type conflicts in shared code
per.liden at oracle.com
Wed Jun 6 21:48:57 UTC 2018
On 06/06/2018 11:15 PM, Chris Phillips wrote:
> Hi Per,
> On 06/06/18 04:47 PM, Per Liden wrote:
>> Hi Chris,
>> On 06/06/2018 09:36 PM, Chris Phillips wrote:
>>> On 06/06/18 02:23 PM, Per Liden wrote:
>>>> On 2018-06-06 18:29, Andrew Haley wrote:
>>>>> On 06/06/2018 04:47 PM, Chris Phillips wrote:
>>>>>> Please review this set of changes to shared code
>>>>>> related to S390 (31bit) Zero self-build type mis-match failures.
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8203030
>>>>>> webrev: http://cr.openjdk.java.net/~chrisphi/JDK-8203030/webrev.0
>>>>> Can you explain this a little more? What is the type of size_t on
>>>>> s390x? What is the type of uintptr_t? What are the errors?
>>>> I would like to understand this too.
>>> Quoting from the original bug review request:
>>> is a problem when one parameter is of size_t type and the second of
>>> uintx type and the platform has size_t defined as eg. unsigned long as
>>> on s390 (32-bit)."
>> Please clarify what the sizes of uintx (i.e. uintptr_t) and size_t are
>> on s390?
> See Dan's explanation.
>> I fail to see how any of this matters to _entries here? What am I missing?
> By changing the type, to its actual usage, we avoid the
> necessity of patching in src/hotspot/share/gc/g1/g1StringDedupTable.cpp
> around line 617, since its consistent usage and local I patched at the
> - _table->_entries, percent_of(_table->_entries, _table->_size),
> _entry_cache->size(), _entries_added, _entries_removed);
> + _table->_entries, percent_of( (size_t)(_table->_entries),
> _table->_size), _entry_cache->size(), _entries_added, _entries_removed);
> percent_of will complain about types otherwise.
Ok, so why don't you just cast it in the call to percent_of? Your
current patch has ripple effects that you fail to take into account. For
example, _entries is still printed using UINTX_FORMAT and compared
against other uintx variables. You're now mixing types in an unsound way.
>> @@ -120,11 +120,11 @@
>> // Cache for reuse and fast alloc/free of table entries.
>> static G1StringDedupEntryCache* _entry_cache;
>> G1StringDedupEntry** _buckets;
>> size_t _size;
>> - uintx _entries;
>> + size_t _entries;
>> uintx _shrink_threshold;
>> uintx _grow_threshold;
>> bool _rehash_needed;
>>> Hope that helps,
>>> (I'll answer further if needed but the info is in the bugs and
>>> review thread mostly)
>>> For more info.
More information about the hotspot-dev