Repository: gfx-rs/wgpu (language: rust)

Issue:
[naga] Constant evaluation doesn't consume `ZeroValue` as intended
The following WGSL should have the return value of `cast_of_splat` fully evaluated as a constant expression:

```
fn cast_of_splat() -> vec4<f32> {
    return vec4<f32>(vec4<i32>(i32()));
}
```

However, `cargo run -p naga-cli -- cast-of-splat.wgsl out.wgsl` produces:

```
fn cast_of_splat() -> vec4<f32> {
    return vec4<f32>(vec4(i32()));
}
```

The body should be: `return vec4<f32>(0.0, 0.0, 0.0, 0.0);`.

This is because `ConstantEvaluator::eval_zero_value_and_splat` promises to lower the given expression to a combination of `Compose` and `Literal` calls, but it calls `splat` to handle `Splat` expressions, which introduces `ZeroValue` expressions.

cc: @teoxoy


Discussion / hints:
As of fd53ea90e675d94f1d79a1c3c44b2f356cecd9c5 the example given above produces this output:

```
fn cast_of_splat() -> vec4<f32> {
    return vec4(0.0);
}
```

@jimblandy can this bug therefore be closed, or is there the issue still present just we need an updated example?
Between a730236b68f0e73f3bafbba767134751e9ea830f and fd53ea90e675d94f1d79a1c3c44b2f356cecd9c5 it seems we're now perfectly happy to allow `Splat` to be a constant evaluated type. So the fact the testcase above now outputs `return vec4(0.0)` (Splat of f32 literal) rather than `return vec4(0.0, 0.0, 0.0, 0.0)` (compose of f32 literals) is absolutely fine.

However, the underlying issue described by Jim is still present, even if the above testcase no longer encounters it:

> This is because ConstantEvaluator::eval_zero_value_and_splat promises to lower the given expression to a combination of Compose and Literal calls, but it calls splat to handle Splat expressions, which introduces ZeroValue expressions.

If we look at places which still call `eval_zero_value_and_splat()` we can construct a testcase. One such caller is `binary_op()`. We additionally need need `eval_zero_value_and_splat()` to call `splat()` in order to reintroduce the zero values, ie we need a Splat of scalar ZeroValues, not a vector ZeroValue. This leads us to:

```wgsl
fn test() -> vec4<f32> {
    return vec4<f32>(5) + vec4<f32>(f32());
}
```

Gives this output:
```wgsl
fn test() -> vec4<f32> {
    return (vec4(5f) + vec4(f32()));
}
```

But it should be able to evaluate away to:
```wgsl
return vec4<f32>(5f, 5f, 5f, 5f);
```
One way we could fix this would be to make `splat()` evaluate zero values to literals then wrap them in a Compose.

Fix patch:
diff --git a/naga/src/proc/constant_evaluator.rs b/naga/src/proc/constant_evaluator.rs
index 42ba489790..c8911077b7 100644
--- a/naga/src/proc/constant_evaluator.rs
+++ b/naga/src/proc/constant_evaluator.rs
@@ -1398,14 +1398,19 @@ impl<'a> ConstantEvaluator<'a> {
     /// [`Compose`]: Expression::Compose
     fn eval_zero_value_and_splat(
         &mut self,
-        expr: Handle<Expression>,
+        mut expr: Handle<Expression>,
         span: Span,
     ) -> Result<Handle<Expression>, ConstantEvaluatorError> {
-        match self.expressions[expr] {
-            Expression::ZeroValue(ty) => self.eval_zero_value_impl(ty, span),
-            Expression::Splat { size, value } => self.splat(value, size, span),
-            _ => Ok(expr),
+        // The result of the splat() for a Splat of a scalar ZeroValue is a
+        // vector ZeroValue, so we must call eval_zero_value_impl() after
+        // splat() in order to ensure we have no ZeroValues remaining.
+        if let Expression::Splat { size, value } = self.expressions[expr] {
+            expr = self.splat(value, size, span)?;
         }
+        if let Expression::ZeroValue(ty) = self.expressions[expr] {
+            expr = self.eval_zero_value_impl(ty, span)?;
+        }
+        Ok(expr)
     }
 
     /// Lower [`ZeroValue`] expressions to [`Literal`] and [`Compose`] expressions.
@@ -2976,4 +2981,84 @@ mod tests {
             panic!("unexpected evaluation result")
         }
     }
+
+    #[test]
+    fn splat_of_zero_value() {
+        let mut types = UniqueArena::new();
+        let constants = Arena::new();
+        let overrides = Arena::new();
+        let mut global_expressions = Arena::new();
+
+        let f32_ty = types.insert(
+            Type {
+                name: None,
+                inner: TypeInner::Scalar(crate::Scalar::F32),
+            },
+            Default::default(),
+        );
+
+        let vec2_f32_ty = types.insert(
+            Type {
+                name: None,
+                inner: TypeInner::Vector {
+                    size: VectorSize::Bi,
+                    scalar: crate::Scalar::F32,
+                },
+            },
+            Default::default(),
+        );
+
+        let five =
+            global_expressions.append(Expression::Literal(Literal::F32(5.0)), Default::default());
+        let five_splat = global_expressions.append(
+            Expression::Splat {
+                size: VectorSize::Bi,
+                value: five,
+            },
+            Default::default(),
+        );
+        let zero = global_expressions.append(Expression::ZeroValue(f32_ty), Default::default());
+        let zero_splat = global_expressions.append(
+            Expression::Splat {
+                size: VectorSize::Bi,
+                value: zero,
+            },
+            Default::default(),
+        );
+
+        let expression_kind_tracker = &mut ExpressionKindTracker::from_arena(&global_expressions);
+        let mut solver = ConstantEvaluator {
+            behavior: Behavior::Wgsl(WgslRestrictions::Const(None)),
+            types: &mut types,
+            constants: &constants,
+            overrides: &overrides,
+            expressions: &mut global_expressions,
+            expression_kind_tracker,
+        };
+
+        let solved_add = solver
+            .try_eval_and_append(
+                Expression::Binary {
+                    op: crate::BinaryOperator::Add,
+                    left: zero_splat,
+                    right: five_splat,
+                },
+                Default::default(),
+            )
+            .unwrap();
+
+        let pass = match global_expressions[solved_add] {
+            Expression::Compose { ty, ref components } => {
+                ty == vec2_f32_ty
+                    && components.iter().all(|&component| {
+                        let component = &global_expressions[component];
+                        matches!(*component, Expression::Literal(Literal::F32(5.0)))
+                    })
+            }
+            _ => false,
+        };
+        if !pass {
+            panic!("unexpected evaluation result")
+        }
+    }
 }


Test patch: