RFR: 8367657: C2 SuperWord: NormalMapping demo from JVMLS 2025 [v3]
Christian Hagedorn
chagedorn at openjdk.org
Tue Sep 16 07:09:03 UTC 2025
On Mon, 15 Sep 2025 17:41:34 GMT, Emanuel Peter <epeter at openjdk.org> wrote:
>> Demo from here:
>> https://inside.java/2025/08/16/jvmls-hotspot-auto-vectorization/
>>
>> Cleaned up and enhanced with a JTREG and IR test.
>> I also added some additional "generated" normal maps from height functions.
>> And I display the resulting image side-by-side with the normal map.
>>
>> I decided to put it in a new directory `compiler.gallery`, anticipating other compiler tests that are both visually appealing (i.e. can be used for a "gallery") and that we may want to back up with other tests like IR testing.
>>
>> There is a **stand-alone** way to run the demo:
>> `java test/hotspot/jtreg/compiler/gallery/NormalMapping.java`
>> (though it may only run with JDK22+, probably due some amber features)
>>
>> Here some snapshots, but **I really recommend pulling the diff and playing with it, it looks much better in motion**:
>> <img width="2000" height="991" alt="image" src="https://github.com/user-attachments/assets/a693fac8-ecf0-43f2-914b-25f76c2f425d" />
>> <img width="2000" height="997" alt="image" src="https://github.com/user-attachments/assets/c2202e6b-6a90-4f90-a3ca-b73304e25905" />
>> <img width="1997" height="992" alt="image" src="https://github.com/user-attachments/assets/0d6da304-6bb9-4b25-9a7b-72019b02d95e" />
>> <img width="1992" height="994" alt="image" src="https://github.com/user-attachments/assets/9f5f7426-0678-45af-a3eb-ac092c262d4c" />
>
> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
>
> fix inlining
Great work and thanks for sharing it! A few small suggestions, otherwise, it looks good to me!
test/hotspot/jtreg/compiler/gallery/NormalMapping.java line 40:
> 38: import java.awt.geom.Path2D;
> 39: import javax.swing.JPanel;
> 40: import java.awt.Font;
Some unused imports (double check again after removing):
Suggestion:
import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.Color;
import java.awt.image.BufferedImage;
import java.awt.image.DataBufferInt;
import java.io.IOException;
import java.util.Random;
import javax.swing.JPanel;
import java.awt.Font;
test/hotspot/jtreg/compiler/gallery/NormalMapping.java line 88:
> 86: System.out.println("Welcome to the Normal Mapping Demo!");
> 87: // Create an applicateion state with 5 lights.
> 88: State state = new State(5);
I suggest to put `5` into a named constant. This invites to play around with different number of lights.
test/hotspot/jtreg/compiler/gallery/NormalMapping.java line 93:
> 91: System.out.println("Setting up Window...");
> 92: MyDrawingPanel panel = new MyDrawingPanel(state);
> 93: JFrame frame = new JFrame("Normal Mapping Demo (Auto Vectorization)");
Suggestion:
JFrame frame = new JFrame("Normal Mapping Demo (Auto-Vectorization)");
test/hotspot/jtreg/compiler/gallery/NormalMapping.java line 121:
> 119: }
> 120:
> 121: public static File getLocalFile(String name) {
Isn't `name` always constant (i.e. `normal_map.png`)? Then you could also extract that to a constant and use it here directly.
test/hotspot/jtreg/compiler/gallery/NormalMapping.java line 149:
> 147: }
> 148:
> 149: public static class Light {
Maybe add a quick comment what this class does since it's a demo and one might want to better understand what's going on. Same for `State` class below.
test/hotspot/jtreg/compiler/gallery/NormalMapping.java line 170:
> 168: dy *= 0.99;
> 169: dx += RANDOM.nextFloat() * 0.001 - 0.0005;;
> 170: dy += RANDOM.nextFloat() * 0.001 - 0.0005;;
Suggestion:
dx += RANDOM.nextFloat() * 0.001 - 0.0005;
dy += RANDOM.nextFloat() * 0.001 - 0.0005;
test/hotspot/jtreg/compiler/gallery/NormalMapping.java line 244:
> 242:
> 243: public void nextNormals() {
> 244: switch(nextNormalsId) {
Suggestion:
switch (nextNormalsId) {
test/hotspot/jtreg/compiler/gallery/NormalMapping.java line 299:
> 297: interface HeightFunction {
> 298: // x and y should be in [0..1]
> 299: public double call(double x, double y);
Implicit:
Suggestion:
double call(double x, double y);
test/hotspot/jtreg/compiler/gallery/NormalMapping.java line 310:
> 308:
> 309: // A selection of "height functions":
> 310: return switch(name) {
Suggestion:
return switch (name) {
test/hotspot/jtreg/compiler/gallery/NormalMapping.java line 314:
> 312: case "heart" -> {
> 313: double heart = Math.abs(Math.pow(x*x + y*y - 1, 3) - x*x * Math.pow(-y, 3));
> 314: double decay = Math.exp(-(x*x + y*y));
Suggestion:
double heart = Math.abs(Math.pow(x * x + y * y - 1, 3) - x * x * Math.pow(-y, 3));
double decay = Math.exp(-(x * x + y * y));
test/hotspot/jtreg/compiler/gallery/NormalMapping.java line 318:
> 316: }
> 317: case "hill" -> 0.5 * Math.exp(-(x*x + y*y));
> 318: case "ripple" -> 0.01 * Math.sin(x*x + y*y);
Suggestion:
case "hill" -> 0.5 * Math.exp(-(x * x + y * y));
case "ripple" -> 0.01 * Math.sin(x * x + y * y);
test/hotspot/jtreg/compiler/gallery/NormalMapping.java line 411:
> 409: for (int i = 0; i < lights.length; i++) {
> 410: lights[i].update();
> 411: }
As below, you could use enhanced-for:
Suggestion:
for (Light light : lights) {
light.update();
}
test/hotspot/jtreg/compiler/gallery/NormalMapping.java line 417:
> 415: Arrays.fill(outputArray, 0);
> 416:
> 417: // Add inn the contribution of each light
Suggestion:
// Add in the contribution of each light
test/hotspot/jtreg/compiler/gallery/NormalMapping.java line 463:
> 461: float luminosity = Math.max(0, dotProduct / d3) * luminosityCorrection;
> 462:
> 463: // Now we we compute the color values that hopefully end up in the range
Suggestion:
// Now we compute the color values that hopefully end up in the range
test/hotspot/jtreg/compiler/gallery/NormalMapping.java line 480:
> 478:
> 479: // This is a bit of a horrible hack, but it mostly works.
> 480: // Essencially, it tries to solve the "exposure" problem:
Suggestion:
// Essentially, it tries to solve the "exposure" problem:
test/hotspot/jtreg/compiler/gallery/TestNormalMapping.java line 29:
> 27: * @summary Visual example of auto vectorization: normal mapping.
> 28: * @library /test/lib /
> 29: * @run main compiler.gallery.TestNormalMapping ir
This should be `driver` because otherwise, we will be stressing the driver VM when run with `Xcomp` etc.
Suggestion:
* @run driver compiler.gallery.TestNormalMapping ir
-------------
PR Review: https://git.openjdk.org/jdk/pull/27282#pullrequestreview-3227811932
PR Review Comment: https://git.openjdk.org/jdk/pull/27282#discussion_r2351083668
PR Review Comment: https://git.openjdk.org/jdk/pull/27282#discussion_r2351084533
PR Review Comment: https://git.openjdk.org/jdk/pull/27282#discussion_r2351069089
PR Review Comment: https://git.openjdk.org/jdk/pull/27282#discussion_r2351078912
PR Review Comment: https://git.openjdk.org/jdk/pull/27282#discussion_r2351088352
PR Review Comment: https://git.openjdk.org/jdk/pull/27282#discussion_r2351085454
PR Review Comment: https://git.openjdk.org/jdk/pull/27282#discussion_r2351093619
PR Review Comment: https://git.openjdk.org/jdk/pull/27282#discussion_r2351098053
PR Review Comment: https://git.openjdk.org/jdk/pull/27282#discussion_r2351098915
PR Review Comment: https://git.openjdk.org/jdk/pull/27282#discussion_r2351101451
PR Review Comment: https://git.openjdk.org/jdk/pull/27282#discussion_r2351102209
PR Review Comment: https://git.openjdk.org/jdk/pull/27282#discussion_r2351110603
PR Review Comment: https://git.openjdk.org/jdk/pull/27282#discussion_r2351104989
PR Review Comment: https://git.openjdk.org/jdk/pull/27282#discussion_r2351112255
PR Review Comment: https://git.openjdk.org/jdk/pull/27282#discussion_r2351112981
PR Review Comment: https://git.openjdk.org/jdk/pull/27282#discussion_r2351131694
More information about the hotspot-compiler-dev
mailing list