RFR: 8350864: C2: verify structural invariants of the Ideal graph [v2]

Marc Chevalier mchevalier at openjdk.org
Thu Jul 31 13:35:58 UTC 2025


On Thu, 31 Jul 2025 10:51:58 GMT, Benoît Maillard <bmaillard at openjdk.org> wrote:

>> Marc Chevalier has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Rename flag as suggested
>
> src/hotspot/share/opto/graphInvariants.hpp line 32:
> 
>> 30: 
>> 31: // An invariant that needs only a local view of the graph, around a given node.
>> 32: class LocalGraphInvariant : public ResourceObj {
> 
> Can't we put the whole definition behind a `#ifndef PRODUCT` check? It seems there are other instances where it is done with classes, such as `VTrace`. Or is there a reason not to?

I had a reason, but disclaimer first, I'm not saying it's a good one.

My idea is that these checks shouldn't be in product, and the flag to enable them is not either. But I also thought it wouldn't be crazy to have this flag as diagnostic one day, or maybe only a diagnostic flag in addition for a subset of checks that we could find useful to have a diagnostic flag for, but that would use the same underlying mechanics.

That being said, it's not going to be the case in a near future, and it's not hard to change a way or the other. So I guess I can put everything behind a `#ifndef PRODUCT` and if we ever regret, the additional work would be really small.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/26362#discussion_r2245418879


More information about the hotspot-compiler-dev mailing list