On Fri, 19 Jun 2026 06:22:16 GMT, Emanuel Peter <[email protected]> wrote:
>> Ashutosh Mehra has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 11 commits: >> >> - Add code missed out when merging master >> >> Signed-off-by: Ashutosh Mehra <[email protected]> >> - Merge branch 'master' into vm_version_x86-reorg >> >> Signed-off-by: Ashutosh Mehra <[email protected]> >> - Address review comment >> >> Signed-off-by: Ashutosh Mehra <[email protected]> >> - Simplify setting of IntelJccErratumMitigation flag >> >> Signed-off-by: Ashutosh Mehra <[email protected]> >> - Fix setting of CopyAVX3Threshold >> >> Signed-off-by: Ashutosh Mehra <[email protected]> >> - Fix another mistake >> >> Signed-off-by: Ashutosh Mehra <[email protected]> >> - Fix build failure >> >> Signed-off-by: Ashutosh Mehra <[email protected]> >> - Replace COMPILER2_OR_JVMCI with COMPILER2 >> >> Signed-off-by: Ashutosh Mehra <[email protected]> >> - Merge branch 'master' into vm_version_x86-reorg >> >> Signed-off-by: Ashutosh Mehra <[email protected]> >> - Minor updates >> >> Signed-off-by: Ashutosh Mehra <[email protected]> >> - ... and 1 more: https://git.openjdk.org/jdk/compare/39d2d165...8821f232 > > src/hotspot/cpu/x86/globals_x86.hpp line 189: > >> 187: constraint(CopyAVX3ThresholdConstraintFunc,AfterErgo) >> \ >> 188: >> \ >> 189: product(bool, IntelJccErratumMitigation, false, DIAGNOSTIC, >> \ > > Drive-by comment. So don't expect me to give a full review ;) > > Is this flag adequately tested? Because if you now make changes, I'm afraid > we might not catch if it breaks. > > The only test I see that sets the flag is this one: > `test/hotspot/jtreg/compiler/c2/irTests/TestPadding.java` > And it only sets the flag to `on`. We should have at least a smoke test that > both sets the flag to on and off. Functionality did not change from original code. It was always reset if default. IntelJccErratumMitigation only adds NOPs to code. And `TestPadding.java` test verifies that code is not broken. On AMD machines it is always off - so we testing this too. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/31301#discussion_r3444244942
