pan: Don't pretend we support load_{vertex_id_zero_base,first_vertex}

load_vertex_id_zero_base() is supposed to return the zero-based
vertex ID, which is then offset by load_first_vertex() to get
an absolute vertex ID. At the same time, when we're in a Vulkan
environment, load_first_vertex() also encodes the vertexOffset
passed to the indexed draw.

Midgard/Bifrost have a sligtly different semantics, where
load_first_vertex() returns vertexOffset + minVertexIdInIndexRange,
and load_vertex_id_zero_base() returns an ID that needs to be offset
by this vertexOffset + minVertexIdInIndexRange to get the absolute
vertex ID. Everything works fine as long as all the load_first_vertex()
and load_vertex_id_zero_base() calls are coming from the
load_vertex_id() lowering. But as mentioned above, that's no longer
the case in Vulkan, where gl_BaseVertexARB will be turned into
load_first_vertex() and expect a value of vertexOffset in an
indexed draw context.

We thus need to fix the mismatch by introducing two new
panfrost-specific intrinsic so we can stop abusing load_first_vertex()
and load_vertex_id_zero_base().

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Chia-I Wu <olvaffe@gmail.com>
Reviewed-by: Lars-Ivar Hesselberg Simonsen <lars-ivar.simonsen@arm.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/32415>

src/compiler/nir/nir_intrinsics.py

@@ -1483,6 +1483,15 @@ intrinsic("prefetch_ubo_ir3", [1], flags=[CAN_REORDER])
 # src[] = { vertex_id, instance_id, offset }
 load("attribute_pan", [1, 1, 1], [BASE, COMPONENT, DEST_TYPE, IO_SEMANTICS], [CAN_ELIMINATE, CAN_REORDER])
 
+# Panfrost-specific intrinsics for accessing the raw vertex ID and the
+# associated offset such that
+#   vertex_id = raw_vertex_id_pan + raw_vertex_offset_pan
+# The raw vertex ID differs from the zero-based vertex ID in that, in an index
+# draw, it is offset by the minimum vertex ID in the index buffer range
+# covered by the draw
+system_value("raw_vertex_id_pan", 1)
+system_value("raw_vertex_offset_pan", 1)
+
 # Intrinsics used by the Midgard/Bifrost blend pipeline. These are defined
 # within a blend shader to read/write the raw value from the tile buffer,
 # without applying any format conversion in the process. If the shader needs

src/gallium/drivers/panfrost/pan_nir_lower_sysvals.c

@@ -86,7 +86,7 @@ sysval_for_intrinsic(unsigned arch, nir_intrinsic_instr *intr, unsigned *offset)
    case nir_intrinsic_load_num_vertices:
       return PAN_SYSVAL_NUM_VERTICES;
 
-   case nir_intrinsic_load_first_vertex:
+   case nir_intrinsic_load_raw_vertex_offset_pan:
       return PAN_SYSVAL_VERTEX_INSTANCE_OFFSETS;
    case nir_intrinsic_load_base_vertex:
       *offset = 4;

src/panfrost/ci/panfrost-g52-fails.txt

@@ -2833,13 +2833,3 @@ dEQP-VK.draw.dynamic_rendering.primary_cmd_buff.inverted_depth_ranges.nodepthcla
 
 dEQP-VK.rasterization.rasterization_order_attachment_access.depth.samples_1.multi_draw_barriers,Crash
 dEQP-VK.rasterization.rasterization_order_attachment_access.stencil.samples_1.multi_draw_barriers,Crash
-
-# new with KHR_shader_draw_parameters
-dEQP-VK.draw.dynamic_rendering.complete_secondary_cmd_buff.shader_draw_parameters.base_instance.draw_indexed,Fail
-dEQP-VK.draw.dynamic_rendering.complete_secondary_cmd_buff.shader_draw_parameters.base_vertex.draw_indexed,Fail
-dEQP-VK.draw.dynamic_rendering.partial_secondary_cmd_buff.shader_draw_parameters.base_instance.draw_indexed,Fail
-dEQP-VK.draw.dynamic_rendering.partial_secondary_cmd_buff.shader_draw_parameters.base_vertex.draw_indexed,Fail
-dEQP-VK.draw.dynamic_rendering.primary_cmd_buff.shader_draw_parameters.base_instance.draw_indexed,Fail
-dEQP-VK.draw.dynamic_rendering.primary_cmd_buff.shader_draw_parameters.base_vertex.draw_indexed,Fail
-dEQP-VK.draw.renderpass.shader_draw_parameters.base_instance.draw_indexed,Fail
-dEQP-VK.draw.renderpass.shader_draw_parameters.base_vertex.draw_indexed,Fail

src/panfrost/compiler/bifrost_compile.c

@@ -1968,10 +1968,12 @@ bi_emit_intrinsic(bi_builder *b, nir_intrinsic_instr *instr)
     * the new flow on Valhall so this is lowered in NIR.
     */
    case nir_intrinsic_load_vertex_id:
-   case nir_intrinsic_load_vertex_id_zero_base:
-      assert(b->shader->malloc_idvs ==
-             (instr->intrinsic == nir_intrinsic_load_vertex_id));
+      assert(b->shader->malloc_idvs);
+      bi_mov_i32_to(b, dst, bi_vertex_id(b));
+      break;
 
+   case nir_intrinsic_load_raw_vertex_id_pan:
+      assert(!b->shader->malloc_idvs);
       bi_mov_i32_to(b, dst, bi_vertex_id(b));
       break;
 
@@ -5173,6 +5175,9 @@ bifrost_preprocess_nir(nir_shader *nir, unsigned gpu_id)
    NIR_PASS(_, nir, nir_lower_vars_to_ssa);
 
    if (nir->info.stage == MESA_SHADER_VERTEX) {
+      if (pan_arch(gpu_id) <= 7)
+         NIR_PASS(_, nir, pan_nir_lower_vertex_id);
+
       NIR_PASS(_, nir, nir_lower_viewport_transform);
       NIR_PASS(_, nir, nir_lower_point_size, 1.0, 0.0);
 

src/panfrost/compiler/bifrost_compile.h

@@ -56,12 +56,6 @@ void bifrost_compile_shader_nir(nir_shader *nir,
       .lower_bitfield_extract = true,                                          \
       .lower_insert_byte = true,                                               \
                                                                                \
-      /* Vertex ID is zero based in the traditional geometry flows, but not in \
-       * the memory-allocated IDVS flow introduced and used exclusively in     \
-       * Valhall. So this is a machine property for us.                        \
-       */                                                                      \
-      .vertex_id_zero_based = (arch <= 7),                                     \
-                                                                               \
       .lower_pack_64_4x16 = true,                                              \
       .lower_pack_half_2x16 = true,                                            \
       .lower_pack_unorm_2x16 = true,                                           \

src/panfrost/lib/pan_shader.c

@@ -128,9 +128,7 @@ GENX(pan_shader_compile)(nir_shader *s, struct panfrost_compile_inputs *inputs,
       info->attribute_count = info->attributes_read_count;
 
 #if PAN_ARCH <= 5
-      bool vertex_id = BITSET_TEST(s->info.system_values_read,
-                                   SYSTEM_VALUE_VERTEX_ID_ZERO_BASE);
-      if (vertex_id)
+      if (info->midgard.vs.reads_raw_vertex_id)
          info->attribute_count = MAX2(info->attribute_count, PAN_VERTEX_ID + 1);
 
       bool instance_id =

src/panfrost/midgard/midgard_compile.c

@@ -377,6 +377,7 @@ midgard_preprocess_nir(nir_shader *nir, unsigned gpu_id)
    NIR_PASS(_, nir, nir_lower_vars_to_ssa);
 
    if (nir->info.stage == MESA_SHADER_VERTEX) {
+      NIR_PASS(_, nir, pan_nir_lower_vertex_id);
       NIR_PASS(_, nir, nir_lower_viewport_transform);
       NIR_PASS(_, nir, nir_lower_point_size, 1.0, 0.0);
    }
@@ -1537,7 +1538,7 @@ static unsigned
 vertex_builtin_arg(nir_intrinsic_op op)
 {
    switch (op) {
-   case nir_intrinsic_load_vertex_id_zero_base:
+   case nir_intrinsic_load_raw_vertex_id_pan:
       return PAN_VERTEX_ID;
    case nir_intrinsic_load_instance_id:
       return PAN_INSTANCE_ID;
@@ -1951,7 +1952,9 @@ emit_intrinsic(compiler_context *ctx, nir_intrinsic_instr *instr)
       emit_compute_builtin(ctx, instr);
       break;
 
-   case nir_intrinsic_load_vertex_id_zero_base:
+   case nir_intrinsic_load_raw_vertex_id_pan:
+      ctx->info->midgard.vs.reads_raw_vertex_id = true;
+      FALLTHROUGH;
    case nir_intrinsic_load_instance_id:
       emit_vertex_builtin(ctx, instr);
       break;

src/panfrost/midgard/midgard_compile.h

@@ -98,7 +98,6 @@ static const nir_shader_compiler_options midgard_nir_options = {
    .has_isub = true,
    .vectorize_io = true,
 
-   .vertex_id_zero_based = true,
    .has_cs_global_id = true,
    .lower_cs_local_index_to_id = true,
    .max_unroll_iterations = 32,

src/panfrost/util/meson.build

@@ -17,6 +17,7 @@ libpanfrost_util_files = files(
   'pan_lower_noperspective.c',
   'pan_lower_sample_position.c',
   'pan_lower_store_component.c',
+  'pan_lower_vertex_id.c',
   'pan_lower_writeout.c',
   'pan_lower_xfb.c',
 )

src/panfrost/util/pan_ir.h

@@ -179,6 +179,11 @@ struct bifrost_shader_info {
 
 struct midgard_shader_info {
    unsigned first_tag;
+   union {
+      struct {
+         bool reads_raw_vertex_id;
+      } vs;
+   };
 };
 
 struct pan_shader_info {
@@ -387,6 +392,8 @@ void pan_print_alu_type(nir_alu_type t, FILE *fp);
 bool pan_nir_lower_zs_store(nir_shader *nir);
 bool pan_nir_lower_store_component(nir_shader *shader);
 
+bool pan_nir_lower_vertex_id(nir_shader *shader);
+
 bool pan_nir_lower_image_ms(nir_shader *shader);
 
 bool pan_nir_lower_frag_coord_zw(nir_shader *shader);

src/panfrost/util/pan_lower_vertex_id.c

@@ -0,0 +1,49 @@
+/*
+ * Copyright (C) 2024 Collabora, Ltd.
+ * SPDX-License-Identifier: MIT
+ */
+
+#include "compiler/nir/nir_builder.h"
+#include "pan_ir.h"
+
+/*
+ * load_vertex_id_zero_base() is supposed to return the zero-based
+ * vertex ID, which is then offset by load_first_vertex() to get
+ * an absolute vertex ID. At the same time, when we're in a Vulkan
+ * environment, load_first_vertex() also encodes the vertexOffset
+ * passed to the indexed draw.
+ *
+ * Midgard/Bifrost have a sligtly different semantics, where
+ * load_first_vertex() returns vertexOffset + minVertexIdInIndexRange,
+ * and load_vertex_id_zero_base() returns an ID that needs to be offset
+ * by this vertexOffset + minVertexIdInIndexRange to get the absolute
+ * vertex ID. Everything works fine as long as all the load_first_vertex()
+ * and load_vertex_id_zero_base() calls are coming from the
+ * load_vertex_id() lowering. But as mentioned above, that's no longer
+ * the case in Vulkan, where gl_BaseVertexARB will be turned into
+ * load_first_vertex() and expect a value of vertexOffset in an
+ * indexed draw context.
+ *
+ * This pass is turning load_vertex_id() calls into
+ * load_raw_vertex_id_pan() + load_raw_vertex_offset_pan().
+ */
+
+static bool
+lower_load_vertex_id(nir_builder *b, nir_intrinsic_instr *intr,
+                     UNUSED void *data)
+{
+   if (intr->intrinsic != nir_intrinsic_load_vertex_id)
+      return false;
+
+   b->cursor = nir_before_instr(&intr->instr);
+   nir_def_replace(&intr->def, nir_iadd(b, nir_load_raw_vertex_id_pan(b),
+                                        nir_load_raw_vertex_offset_pan(b)));
+   return true;
+}
+
+bool
+pan_nir_lower_vertex_id(nir_shader *shader)
+{
+   return nir_shader_intrinsics_pass(shader, lower_load_vertex_id,
+                                     nir_metadata_control_flow, NULL);
+}

src/panfrost/util/pan_lower_xfb.c

@@ -40,7 +40,7 @@ lower_xfb_output(nir_builder *b, nir_intrinsic_instr *intr,
 
    nir_def *index = nir_iadd(
       b, nir_imul(b, nir_load_instance_id(b), nir_load_num_vertices(b)),
-      nir_load_vertex_id_zero_base(b));
+      nir_load_raw_vertex_id_pan(b));
 
    BITSET_SET(b->shader->info.system_values_read,
               SYSTEM_VALUE_VERTEX_ID_ZERO_BASE);
@@ -66,8 +66,8 @@ lower_xfb(nir_builder *b, nir_intrinsic_instr *intr, UNUSED void *data)
    if (intr->intrinsic == nir_intrinsic_load_vertex_id) {
       b->cursor = nir_instr_remove(&intr->instr);
 
-      nir_def *repl =
-         nir_iadd(b, nir_load_vertex_id_zero_base(b), nir_load_first_vertex(b));
+      nir_def *repl = nir_iadd(b, nir_load_raw_vertex_id_pan(b),
+                               nir_load_raw_vertex_offset_pan(b));
 
       nir_def_rewrite_uses(&intr->def, repl);
       return true;

src/panfrost/vulkan/jm/panvk_vX_cmd_draw.c

@@ -97,12 +97,16 @@ panvk_cmd_prepare_draw_sysvals(struct panvk_cmd_buffer *cmdbuf,
    struct panvk_graphics_sysvals *sysvals = &cmdbuf->state.gfx.sysvals;
    struct vk_color_blend_state *cb = &cmdbuf->vk.dynamic_graphics_state.cb;
 
+   int32_t first_vertex =
+      draw->index_size ? draw->vertex_offset : draw->first_vertex;
    uint32_t noperspective_varyings = fs ? fs->info.varyings.noperspective : 0;
-   if (sysvals->vs.first_vertex != draw->offset_start ||
+   if (sysvals->vs.raw_vertex_offset != draw->offset_start ||
+       sysvals->vs.first_vertex != first_vertex ||
        sysvals->vs.base_instance != draw->first_instance ||
        sysvals->layer_id != draw->layer_id ||
        sysvals->vs.noperspective_varyings != noperspective_varyings) {
-      sysvals->vs.first_vertex = draw->offset_start;
+      sysvals->vs.raw_vertex_offset = draw->offset_start;
+      sysvals->vs.first_vertex = first_vertex;
       sysvals->vs.base_instance = draw->first_instance;
       sysvals->vs.noperspective_varyings = noperspective_varyings;
       sysvals->layer_id = draw->layer_id;

src/panfrost/vulkan/panvk_shader.h

@@ -63,6 +63,9 @@ struct panvk_graphics_sysvals {
    } blend;
 
    struct {
+#if PAN_ARCH <= 7
+      int32_t raw_vertex_offset;
+#endif
       int32_t first_vertex;
       int32_t base_instance;
       uint32_t noperspective_varyings;

src/panfrost/vulkan/panvk_vX_shader.c

@@ -100,6 +100,9 @@ panvk_lower_sysvals(nir_builder *b, nir_instr *instr, void *data)
       break;
 
 #if PAN_ARCH <= 7
+   case nir_intrinsic_load_raw_vertex_offset_pan:
+      val = load_sysval(b, graphics, bit_size, vs.raw_vertex_offset);
+      break;
    case nir_intrinsic_load_layer_id:
       assert(b->shader->info.stage == MESA_SHADER_FRAGMENT);
       val = load_sysval(b, graphics, bit_size, layer_id);
@@ -135,8 +138,8 @@ panvk_lower_load_vs_input(nir_builder *b, nir_intrinsic_instr *intrin,
    b->cursor = nir_before_instr(&intrin->instr);
    nir_def *ld_attr = nir_load_attribute_pan(
       b, intrin->def.num_components, intrin->def.bit_size,
-      b->shader->options->vertex_id_zero_based ?
-         nir_load_vertex_id_zero_base(b) :
+      PAN_ARCH <= 7 ?
+         nir_load_raw_vertex_id_pan(b) :
          nir_load_vertex_id(b),
       PAN_ARCH >= 9 ?
          nir_iadd(b, nir_load_instance_id(b), nir_load_base_instance(b)) :