[naga spv-out msl-out hlsl-out] Make infinite loop workaround count down instead of up (#7372)

[naga spv-out msl-out hlsl-out] Make infinite loop workaround count down instead of up

To avoid generating code containing infinite loops, and therefore
incurring the wrath of undefined behaviour, we insert a counter into
each loop that will break after 2^64 iterations. This was previously
implemented as two u32 variables counting up from zero.

We have been informed that this construct can cause certain Intel
drivers to hang. Instead, we must count down from u32::MAX. Counting
down is more fun, anyway.

Co-authored-by: Erich Gubler <erichdongubler@gmail.com>

Author: jamienicol

naga/src/back/hlsl/writer.rs

@@ -170,12 +170,14 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
         }
 
         let loop_bound_name = self.namer.call("loop_bound");
-        let decl = format!("{level}uint2 {loop_bound_name} = uint2(0u, 0u);");
-        let level = level.next();
         let max = u32::MAX;
+        // Count down from u32::MAX rather than up from 0 to avoid hang on
+        // certain Intel drivers. See <https://github.com/gfx-rs/wgpu/issues/7319>.
+        let decl = format!("{level}uint2 {loop_bound_name} = uint2({max}u, {max}u);");
+        let level = level.next();
         let break_and_inc = format!(
-            "{level}if (all({loop_bound_name} == uint2({max}u, {max}u))) {{ break; }}
-{level}{loop_bound_name} += uint2({loop_bound_name}.y == {max}u, 1u);"
+            "{level}if (all({loop_bound_name} == uint2(0u, 0u))) {{ break; }}
+{level}{loop_bound_name} -= uint2({loop_bound_name}.y == 0u, 1u);"
         );
 
         Some((decl, break_and_inc))

naga/src/back/msl/writer.rs

@@ -850,12 +850,13 @@ impl<W: Write> Writer<W> {
         }
 
         let loop_bound_name = self.namer.call("loop_bound");
-        let decl = format!("{level}uint2 {loop_bound_name} = uint2(0u);");
+        // Count down from u32::MAX rather than up from 0 to avoid hang on
+        // certain Intel drivers. See <https://github.com/gfx-rs/wgpu/issues/7319>.
+        let decl = format!("{level}uint2 {loop_bound_name} = uint2({}u);", u32::MAX);
         let level = level.next();
-        let max = u32::MAX;
         let break_and_inc = format!(
-            "{level}if ({NAMESPACE}::all({loop_bound_name} == uint2({max}u))) {{ break; }}
-{level}{loop_bound_name} += uint2({loop_bound_name}.y == {max}u, 1u);"
+            "{level}if ({NAMESPACE}::all({loop_bound_name} == uint2(0u))) {{ break; }}
+{level}{loop_bound_name} -= uint2({loop_bound_name}.y == 0u, 1u);"
         );
 
         Some((decl, break_and_inc))

naga/src/back/spv/block.rs

@@ -310,7 +310,7 @@ impl BlockContext<'_> {
                 uint2_ptr_type_id,
                 loop_counter_var_id,
                 spirv::StorageClass::Function,
-                Some(zero_uint2_const_id),
+                Some(max_uint2_const_id),
             ),
         };
         self.function.force_loop_bounding_vars.push(var);
@@ -331,14 +331,14 @@ impl BlockContext<'_> {
             None,
         ));
 
-        // If both the high and low u32s have reached u32::MAX then break. ie
-        // if (all(eq(loop_counter, vec2(u32::MAX)))) { break; }
+        // If both the high and low u32s have reached 0 then break. ie
+        // if (all(eq(loop_counter, vec2(0)))) { break; }
         let eq_id = self.gen_id();
         block.body.push(Instruction::binary(
             spirv::Op::IEqual,
             bool2_type_id,
             eq_id,
-            max_uint2_const_id,
+            zero_uint2_const_id,
             load_id,
         ));
         let all_eq_id = self.gen_id();
@@ -360,9 +360,11 @@ impl BlockContext<'_> {
         );
         block = Block::new(inc_counter_block_id);
 
-        // To simulate a 64-bit counter we always increment the low u32, and increment
+        // To simulate a 64-bit counter we always decrement the low u32, and decrement
         // the high u32 when the low u32 overflows. ie
-        // counter += vec2(select(0u, 1u, counter.y == u32::MAX), 1u);
+        // counter -= vec2(select(0u, 1u, counter.y == 0), 1u);
+        // Count down from u32::MAX rather than up from 0 to avoid hang on
+        // certain Intel drivers. See <https://github.com/gfx-rs/wgpu/issues/7319>.
         let low_id = self.gen_id();
         block.body.push(Instruction::composite_extract(
             uint_type_id,
@@ -376,7 +378,7 @@ impl BlockContext<'_> {
             bool_type_id,
             low_overflow_id,
             low_id,
-            max_uint_const_id,
+            zero_uint_const_id,
         ));
         let carry_bit_id = self.gen_id();
         block.body.push(Instruction::select(
@@ -386,19 +388,19 @@ impl BlockContext<'_> {
             one_uint_const_id,
             zero_uint_const_id,
         ));
-        let increment_id = self.gen_id();
+        let decrement_id = self.gen_id();
         block.body.push(Instruction::composite_construct(
             uint2_type_id,
-            increment_id,
+            decrement_id,
             &[carry_bit_id, one_uint_const_id],
         ));
         let result_id = self.gen_id();
         block.body.push(Instruction::binary(
-            spirv::Op::IAdd,
+            spirv::Op::ISub,
             uint2_type_id,
             result_id,
             load_id,
-            increment_id,
+            decrement_id,
         ));
         block
             .body

naga/tests/out/hlsl/boids.hlsl

@@ -41,11 +41,11 @@ void main(uint3 global_invocation_id : SV_DispatchThreadID)
     vPos = _e8;
     float2 _e14 = asfloat(particlesSrc.Load2(8+index*16+0));
     vVel = _e14;
-    uint2 loop_bound = uint2(0u, 0u);
+    uint2 loop_bound = uint2(4294967295u, 4294967295u);
     bool loop_init = true;
     while(true) {
-        if (all(loop_bound == uint2(4294967295u, 4294967295u))) { break; }
-        loop_bound += uint2(loop_bound.y == 4294967295u, 1u);
+        if (all(loop_bound == uint2(0u, 0u))) { break; }
+        loop_bound -= uint2(loop_bound.y == 0u, 1u);
         if (!loop_init) {
             uint _e91 = i;
             i = (_e91 + 1u);

naga/tests/out/hlsl/break-if.hlsl

@@ -1,10 +1,10 @@
 void breakIfEmpty()
 {
-    uint2 loop_bound = uint2(0u, 0u);
+    uint2 loop_bound = uint2(4294967295u, 4294967295u);
     bool loop_init = true;
     while(true) {
-        if (all(loop_bound == uint2(4294967295u, 4294967295u))) { break; }
-        loop_bound += uint2(loop_bound.y == 4294967295u, 1u);
+        if (all(loop_bound == uint2(0u, 0u))) { break; }
+        loop_bound -= uint2(loop_bound.y == 0u, 1u);
         if (!loop_init) {
             if (true) {
                 break;
@@ -20,11 +20,11 @@ void breakIfEmptyBody(bool a)
     bool b = (bool)0;
     bool c = (bool)0;
 
-    uint2 loop_bound_1 = uint2(0u, 0u);
+    uint2 loop_bound_1 = uint2(4294967295u, 4294967295u);
     bool loop_init_1 = true;
     while(true) {
-        if (all(loop_bound_1 == uint2(4294967295u, 4294967295u))) { break; }
-        loop_bound_1 += uint2(loop_bound_1.y == 4294967295u, 1u);
+        if (all(loop_bound_1 == uint2(0u, 0u))) { break; }
+        loop_bound_1 -= uint2(loop_bound_1.y == 0u, 1u);
         if (!loop_init_1) {
             b = a;
             bool _e2 = b;
@@ -44,11 +44,11 @@ void breakIf(bool a_1)
     bool d = (bool)0;
     bool e = (bool)0;
 
-    uint2 loop_bound_2 = uint2(0u, 0u);
+    uint2 loop_bound_2 = uint2(4294967295u, 4294967295u);
     bool loop_init_2 = true;
     while(true) {
-        if (all(loop_bound_2 == uint2(4294967295u, 4294967295u))) { break; }
-        loop_bound_2 += uint2(loop_bound_2.y == 4294967295u, 1u);
+        if (all(loop_bound_2 == uint2(0u, 0u))) { break; }
+        loop_bound_2 -= uint2(loop_bound_2.y == 0u, 1u);
         if (!loop_init_2) {
             bool _e5 = e;
             if ((a_1 == _e5)) {
@@ -67,11 +67,11 @@ void breakIfSeparateVariable()
 {
     uint counter = 0u;
 
-    uint2 loop_bound_3 = uint2(0u, 0u);
+    uint2 loop_bound_3 = uint2(4294967295u, 4294967295u);
     bool loop_init_3 = true;
     while(true) {
-        if (all(loop_bound_3 == uint2(4294967295u, 4294967295u))) { break; }
-        loop_bound_3 += uint2(loop_bound_3.y == 4294967295u, 1u);
+        if (all(loop_bound_3 == uint2(0u, 0u))) { break; }
+        loop_bound_3 -= uint2(loop_bound_3.y == 0u, 1u);
         if (!loop_init_3) {
             uint _e5 = counter;
             if ((_e5 == 5u)) {

naga/tests/out/hlsl/collatz.hlsl

@@ -14,10 +14,10 @@ uint collatz_iterations(uint n_base)
     uint i = 0u;
 
     n = n_base;
-    uint2 loop_bound = uint2(0u, 0u);
+    uint2 loop_bound = uint2(4294967295u, 4294967295u);
     while(true) {
-        if (all(loop_bound == uint2(4294967295u, 4294967295u))) { break; }
-        loop_bound += uint2(loop_bound.y == 4294967295u, 1u);
+        if (all(loop_bound == uint2(0u, 0u))) { break; }
+        loop_bound -= uint2(loop_bound.y == 0u, 1u);
         uint _e4 = n;
         if ((_e4 > 1u)) {
         } else {

naga/tests/out/hlsl/control-flow.hlsl

@@ -64,10 +64,10 @@ void switch_const_expr_case_selectors()
 
 void loop_switch_continue(int x)
 {
-    uint2 loop_bound = uint2(0u, 0u);
+    uint2 loop_bound = uint2(4294967295u, 4294967295u);
     while(true) {
-        if (all(loop_bound == uint2(4294967295u, 4294967295u))) { break; }
-        loop_bound += uint2(loop_bound.y == 4294967295u, 1u);
+        if (all(loop_bound == uint2(0u, 0u))) { break; }
+        loop_bound -= uint2(loop_bound.y == 0u, 1u);
         bool should_continue = false;
         switch(x) {
             case 1: {
@@ -87,10 +87,10 @@ void loop_switch_continue(int x)
 
 void loop_switch_continue_nesting(int x_1, int y, int z)
 {
-    uint2 loop_bound_1 = uint2(0u, 0u);
+    uint2 loop_bound_1 = uint2(4294967295u, 4294967295u);
     while(true) {
-        if (all(loop_bound_1 == uint2(4294967295u, 4294967295u))) { break; }
-        loop_bound_1 += uint2(loop_bound_1.y == 4294967295u, 1u);
+        if (all(loop_bound_1 == uint2(0u, 0u))) { break; }
+        loop_bound_1 -= uint2(loop_bound_1.y == 0u, 1u);
         bool should_continue_1 = false;
         switch(x_1) {
             case 1: {
@@ -104,10 +104,10 @@ void loop_switch_continue_nesting(int x_1, int y, int z)
                         break;
                     }
                     default: {
-                        uint2 loop_bound_2 = uint2(0u, 0u);
+                        uint2 loop_bound_2 = uint2(4294967295u, 4294967295u);
                         while(true) {
-                            if (all(loop_bound_2 == uint2(4294967295u, 4294967295u))) { break; }
-                            loop_bound_2 += uint2(loop_bound_2.y == 4294967295u, 1u);
+                            if (all(loop_bound_2 == uint2(0u, 0u))) { break; }
+                            loop_bound_2 -= uint2(loop_bound_2.y == 0u, 1u);
                             bool should_continue_2 = false;
                             switch(z) {
                                 case 1: {
@@ -146,10 +146,10 @@ void loop_switch_continue_nesting(int x_1, int y, int z)
             continue;
         }
     }
-    uint2 loop_bound_3 = uint2(0u, 0u);
+    uint2 loop_bound_3 = uint2(4294967295u, 4294967295u);
     while(true) {
-        if (all(loop_bound_3 == uint2(4294967295u, 4294967295u))) { break; }
-        loop_bound_3 += uint2(loop_bound_3.y == 4294967295u, 1u);
+        if (all(loop_bound_3 == uint2(0u, 0u))) { break; }
+        loop_bound_3 -= uint2(loop_bound_3.y == 0u, 1u);
         bool should_continue_4 = false;
         do {
             do {
@@ -171,10 +171,10 @@ void loop_switch_omit_continue_variable_checks(int x_2, int y_1, int z_1, int w)
 {
     int pos_1 = int(0);
 
-    uint2 loop_bound_4 = uint2(0u, 0u);
+    uint2 loop_bound_4 = uint2(4294967295u, 4294967295u);
     while(true) {
-        if (all(loop_bound_4 == uint2(4294967295u, 4294967295u))) { break; }
-        loop_bound_4 += uint2(loop_bound_4.y == 4294967295u, 1u);
+        if (all(loop_bound_4 == uint2(0u, 0u))) { break; }
+        loop_bound_4 -= uint2(loop_bound_4.y == 0u, 1u);
         bool should_continue_5 = false;
         switch(x_2) {
             case 1: {
@@ -186,10 +186,10 @@ void loop_switch_omit_continue_variable_checks(int x_2, int y_1, int z_1, int w)
             }
         }
     }
-    uint2 loop_bound_5 = uint2(0u, 0u);
+    uint2 loop_bound_5 = uint2(4294967295u, 4294967295u);
     while(true) {
-        if (all(loop_bound_5 == uint2(4294967295u, 4294967295u))) { break; }
-        loop_bound_5 += uint2(loop_bound_5.y == 4294967295u, 1u);
+        if (all(loop_bound_5 == uint2(0u, 0u))) { break; }
+        loop_bound_5 -= uint2(loop_bound_5.y == 0u, 1u);
         bool should_continue_6 = false;
         switch(x_2) {
             case 1: {

naga/tests/out/hlsl/do-while.hlsl

@@ -1,10 +1,10 @@
 void fb1_(inout bool cond)
 {
-    uint2 loop_bound = uint2(0u, 0u);
+    uint2 loop_bound = uint2(4294967295u, 4294967295u);
     bool loop_init = true;
     while(true) {
-        if (all(loop_bound == uint2(4294967295u, 4294967295u))) { break; }
-        loop_bound += uint2(loop_bound.y == 4294967295u, 1u);
+        if (all(loop_bound == uint2(0u, 0u))) { break; }
+        loop_bound -= uint2(loop_bound.y == 0u, 1u);
         if (!loop_init) {
             bool _e1 = cond;
             if (!(_e1)) {

naga/tests/out/hlsl/ray-query.hlsl

@@ -84,10 +84,10 @@ RayIntersection query_loop(float3 pos, float3 dir, RaytracingAccelerationStructu
     RayQuery<RAY_FLAG_NONE> rq_1;
 
     rq_1.TraceRayInline(acs, ConstructRayDesc_(4u, 255u, 0.1, 100.0, pos, dir).flags, ConstructRayDesc_(4u, 255u, 0.1, 100.0, pos, dir).cull_mask, RayDescFromRayDesc_(ConstructRayDesc_(4u, 255u, 0.1, 100.0, pos, dir)));
-    uint2 loop_bound = uint2(0u, 0u);
+    uint2 loop_bound = uint2(4294967295u, 4294967295u);
     while(true) {
-        if (all(loop_bound == uint2(4294967295u, 4294967295u))) { break; }
-        loop_bound += uint2(loop_bound.y == 4294967295u, 1u);
+        if (all(loop_bound == uint2(0u, 0u))) { break; }
+        loop_bound -= uint2(loop_bound.y == 0u, 1u);
         const bool _e9 = rq_1.Proceed();
         if (_e9) {
         } else {

naga/tests/out/hlsl/shadow.hlsl

@@ -95,11 +95,11 @@ float4 fs_main(FragmentInput_fs_main fragmentinput_fs_main) : SV_Target0
     uint i = 0u;
 
     float3 normal_1 = normalize(in_.world_normal);
-    uint2 loop_bound = uint2(0u, 0u);
+    uint2 loop_bound = uint2(4294967295u, 4294967295u);
     bool loop_init = true;
     while(true) {
-        if (all(loop_bound == uint2(4294967295u, 4294967295u))) { break; }
-        loop_bound += uint2(loop_bound.y == 4294967295u, 1u);
+        if (all(loop_bound == uint2(0u, 0u))) { break; }
+        loop_bound -= uint2(loop_bound.y == 0u, 1u);
         if (!loop_init) {
             uint _e40 = i;
             i = (_e40 + 1u);
@@ -134,11 +134,11 @@ float4 fs_main_without_storage(FragmentInput_fs_main_without_storage fragmentinp
     uint i_1 = 0u;
 
     float3 normal_2 = normalize(in_1.world_normal);
-    uint2 loop_bound_1 = uint2(0u, 0u);
+    uint2 loop_bound_1 = uint2(4294967295u, 4294967295u);
     bool loop_init_1 = true;
     while(true) {
-        if (all(loop_bound_1 == uint2(4294967295u, 4294967295u))) { break; }
-        loop_bound_1 += uint2(loop_bound_1.y == 4294967295u, 1u);
+        if (all(loop_bound_1 == uint2(0u, 0u))) { break; }
+        loop_bound_1 -= uint2(loop_bound_1.y == 0u, 1u);
         if (!loop_init_1) {
             uint _e40 = i_1;
             i_1 = (_e40 + 1u);