In `javax.swing.JComponent::_paintImmediately`, in `src/java.desktop/share/classes/javax/swing/JComponent.java`, because few classes are loaded during CTW, `isOptimizedDrawingEnabled()` is believed to be always true (because no overload is known), so https://github.com/openjdk/valhalla/blob/e391f76e7db834f50c1a0af466daf95a65a79f23/src/java.desktop/share/classes/javax/swing/JComponent.java#L5123 seems impossible, which is the only path that allows to re-assign `paintingComponent`. Few steps later, we meet the comparison https://github.com/openjdk/valhalla/blob/e391f76e7db834f50c1a0af466daf95a65a79f23/src/java.desktop/share/classes/javax/swing/JComponent.java#L5213 and then, `paintingComponent` is known to be `this` (still under the assumption there are no overloads of `isOptimizedDrawingEnabled`).
Overall, C1 is right to believe we are comparing `this` with itself. So, in https://github.com/openjdk/valhalla/blob/e391f76e7db834f50c1a0af466daf95a65a79f23/src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp#L1636 `left` and `right` are the same thing. In the callee, if `UseCompactObjectHeaders`: https://github.com/openjdk/valhalla/blob/e391f76e7db834f50c1a0af466daf95a65a79f23/src/hotspot/cpu/x86/macroAssembler_x86.cpp#L5722-L5733 we assert that `left` (`tmp1`) and `right` (`tmp2`) are distinct (among other). This doesn't sound very necessary, but on the other hand, the test is rather redundant if `tmp1 == tmp2`. It is also weird we aren't doing this test in the `else` branch. The non-COH case is a bit cheaper, and so a bit less bad to have uselessly, but it doesn't sound like a very convincing reason. Let's say it's because of history. We could remove the assert, but can save from a useless class comparison. Also, the callee is older than the caller. I suggest we simply bypass the test in the caller if `left == right`. In this case, it is obvious the test would be successful, and we can just emit an unconditional jump. Another option would be to shortcut the whole function by adding if (left == right) { move(op->equal_result(), op->result_opr()); return; } immediately after https://github.com/openjdk/valhalla/blob/e391f76e7db834f50c1a0af466daf95a65a79f23/src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp#L1599-L1600 but then we don't run https://github.com/openjdk/valhalla/blob/e391f76e7db834f50c1a0af466daf95a65a79f23/src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp#L1652 and so we hit this assert https://github.com/openjdk/valhalla/blob/e391f76e7db834f50c1a0af466daf95a65a79f23/src/hotspot/share/c1/c1_LIRAssembler.cpp#L157 Maybe if (left == right) { __ bind(*op->stub()->continuation()); // Just to bind it move(op->equal_result(), op->result_opr()); return; } would be enough, but I'm scared to break a subtle invariant somewhere about stubs, by not going in. Moreover, because of https://github.com/openjdk/valhalla/blob/e391f76e7db834f50c1a0af466daf95a65a79f23/src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp#L1602-L1603 we already don't execute any of this code at runtime. The runtime gain is minimal, so I tend toward stability (it's just C1 after all...), at least for now, with Valhalla integration getting close. I suppose a better improvement might be to simply fold the substitutability check entirely, but that might be too much for C1's limited responsibilities and simple behavior. Thanks, Marc --------- - [x] I confirm that I make this contribution in accordance with the [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). ------------- Commit messages: - fix Changes: https://git.openjdk.org/valhalla/pull/2359/files Webrev: https://webrevs.openjdk.org/?repo=valhalla&pr=2359&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8382776 Stats: 6 lines in 1 file changed: 4 ins; 0 del; 2 mod Patch: https://git.openjdk.org/valhalla/pull/2359.diff Fetch: git fetch https://git.openjdk.org/valhalla.git pull/2359/head:pull/2359 PR: https://git.openjdk.org/valhalla/pull/2359
