RFR: 8213416: Replace some enums with static const members in hotspot/compiler
rahul.v.raghavan at oracle.com
Mon May 20 18:16:10 UTC 2019
With reference to below email thread, request help to confirm next steps
So may I go ahead with webrev changes related to hotspot/compiler for
<webrev.01> - http://cr.openjdk.java.net/~rraghavan/8213416/webrev.01/
(also will add similar hotspot/runtime related details in JBS comments
On 16/05/19 3:26 PM, Rahul Raghavan wrote:> Hi,
> Thank you David for review comments.
> I will kindly request help from Magnus to reply for the main questions.
> Sharing some notes, related links -
> - 8211073: Remove -Wno-extra from Hotspot
> - Discussions in earlier thread -
> So understood -Wextra do help in catching valid/useful warnings also,
> but along with some too strict ones like "enumeral and non-enumeral type
> in conditional expression" type warnings.
> Extracts from 8211073 JBS comments from Magnus regarding the
> 'enum-warning' -
> "... If you think that gcc is a bit too picky here, I agree. It's not
> obvious per se that the added casts improve the code. However, this is
> the price we need to pay to be able to enable -Wextra, and *that* is
> something that is likely to improve the code."
> On 16/05/19 11:13 AM, David Holmes wrote:
>> This all seems like unnecessary churn to me - is any of this code
>> actually wrong? can we not just disable this particular warning? is
>> there any point using "static const" when we should be aiming to use
>> C++11 constexpr in the (not too distant?) future?
>> Converting from enums to unrelated ints seems a big step backwards in
>> software engineering terms. :(
On 16/05/19 12:03 PM, Rahul Raghavan wrote:
> Thank you Vladimir for review comments.
>>> 4) _lh_array_tag_obj_value, _lh_instance_slow_path_bit -
>> I am okay with it but Runtime group should agree too - it is their code.
> Yes, I missed that it is Runtime code.
> Please note plan is to handle only the hotspot/compiler part of the
> requirement changes in JDK-8213416.
> As per earlier JBS comments new JDK-8223400 was created to cover the
> requirements in hotspot/runtime.
> So may I suggest moving the above runtime change requirement details to
> and use only the balance changes, as in below updated webrev, here for
> <webrev.01> - http://cr.openjdk.java.net/~rraghavan/8213416/webrev.01/
More information about the hotspot-compiler-dev