[virglrenderer-devel] [PATCH 02/10] shader: add image support to shader parsing. (v2.1)
Dave Airlie
airlied at gmail.com
Mon Jul 30 04:36:11 UTC 2018
>> #define MAX_IMMEDIATE 1024
>> struct immed {
>> int type;
>> @@ -143,6 +152,9 @@ struct dump_ctx {
>> uint32_t ssbo_array_base;
>> uint32_t ssbo_atomic_array_base;
>>
>> + struct vrend_shader_image images[32];
>> + uint32_t images_used_mask;
>> +
>> struct vrend_sampler_array *sampler_arrays;
>> uint32_t num_sampler_arrays;
>> int last_sampler_array_idx;
>> @@ -210,6 +222,9 @@ static const struct vrend_shader_table
>> shader_req_table[] = {
>> { SHADER_REQ_GPU_SHADER5, "GL_ARB_gpu_shader5" },
>> { SHADER_REQ_DERIVATIVE_CONTROL, "GL_ARB_derivative_control" },
>> { SHADER_REQ_FP64, "GL_ARB_gpu_shader_fp64" },
>> + { SHADER_REQ_IMAGE_LOAD_STORE, "GL_ARB_shader_image_load_store"
>> },
>> + { SHADER_REQ_ES31_COMPAT, "GL_ARB_ES3_1_compatibility" },
>> + { SHADER_REQ_IMAGE_SIZE, "GL_ARB_shader_image_size" },
>> };
> For later: that looks like a candidate that could make use of the new
> feature table (i.e. replace the extension strings by feat_* and test
> for that).
This is kinda the reverse of that, this tells us which strings the shader
needs to contain for the SHADER_REQ, I'm not sure it makes much
sense to reuse the feature stuff here, except maybe we should confirm
we have the features for certain shaders, but since we don't segfault
or crash on bad tgsi we'd just report an error and the rogue guest
would be in the same place.
>> }
>>
>> +static enum vrend_type_qualifier get_coord_prefix(int resource, bool
>> *is_ms)
>> +{
>> + switch(resource) {
>> + case TGSI_TEXTURE_1D:
>> + case TGSI_TEXTURE_BUFFER:
>> + return INT;
>> + case TGSI_TEXTURE_2D:
>> + case TGSI_TEXTURE_RECT:
>> + case TGSI_TEXTURE_1D_ARRAY:
>> + return IVEC2;
>> + case TGSI_TEXTURE_3D:
>> + case TGSI_TEXTURE_CUBE:
>> + case TGSI_TEXTURE_2D_ARRAY:
>> + case TGSI_TEXTURE_CUBE_ARRAY:
>> + return IVEC3;
>> + case TGSI_TEXTURE_2D_MSAA:
>> + *is_ms = true;
>> + return IVEC2;
>> + case TGSI_TEXTURE_2D_ARRAY_MSAA:
>> + *is_ms = true;
>> + return IVEC3;
>> + default:
>> + fprintf(stderr, "NON 2D IMAGE\n");
>> + return TYPE_CONVERSION_NONE;
> Nit: 1D and BUFFER are also not "2D IMAGE"
Will drop the fprintf, was a debug bit from a while back.
>
>> + }
>> +}
>> +
>> static int
>> +const char *get_internalformat_string(int virgl_format, enum
>> tgsi_return_type *stype)
>> +{
>> + switch (virgl_format) {
>> + case PIPE_FORMAT_R11G11B10_FLOAT:
>> + *stype = TGSI_RETURN_TYPE_FLOAT;
>> + return "layout(r11f_g11f_b10f) ";
>> + case PIPE_FORMAT_R10G10B10A2_UNORM:
>> + *stype = TGSI_RETURN_TYPE_UNORM;
>> + return "layout(rgb10_a2) ";
>> + case PIPE_FORMAT_R10G10B10A2_UINT:
>> + *stype = TGSI_RETURN_TYPE_UINT;
>> + return "layout(rgb10_a2ui) ";
>> + case PIPE_FORMAT_R8_UNORM:
>> + *stype = TGSI_RETURN_TYPE_UNORM;
>> + return "layout(r8) ";
>> + case PIPE_FORMAT_R8_SNORM:
>> + *stype = TGSI_RETURN_TYPE_SNORM;
>> + return "layout(r8_snorm) ";
>> + case PIPE_FORMAT_R8_UINT:
>> + *stype = TGSI_RETURN_TYPE_UINT;
>> + return "layout(r8ui) ";
>> + case PIPE_FORMAT_R8_SINT:
>> + *stype = TGSI_RETURN_TYPE_SINT;
>> + return "layout(r8i) ";
>> + case PIPE_FORMAT_R8G8_UNORM:
>> + *stype = TGSI_RETURN_TYPE_UNORM;
>> + return "layout(rg8) ";
>> + case PIPE_FORMAT_R8G8_SNORM:
>> + *stype = TGSI_RETURN_TYPE_SNORM;
>> + return "layout(rg8_snorm) ";
>> + case PIPE_FORMAT_R8G8_UINT:
>> + *stype = TGSI_RETURN_TYPE_UINT;
>> + return "layout(rg8ui) ";
>> + case PIPE_FORMAT_R8G8_SINT:
>> + *stype = TGSI_RETURN_TYPE_SINT;
>> + return "layout(rg8i) ";
>> + case PIPE_FORMAT_R8G8B8A8_UNORM:
>> + *stype = TGSI_RETURN_TYPE_UNORM;
>> + return "layout(rgba8) ";
>> + case PIPE_FORMAT_R8G8B8A8_SNORM:
>> + *stype = TGSI_RETURN_TYPE_SNORM;
>> + return "layout(rgba8_snorm) ";
>> + case PIPE_FORMAT_R8G8B8A8_UINT:
>> + *stype = TGSI_RETURN_TYPE_UINT;
>> + return "layout(rgba8ui) ";
>> + case PIPE_FORMAT_R8G8B8A8_SINT:
>> + *stype = TGSI_RETURN_TYPE_SINT;
>> + return "layout(rgba8i) ";
>> + case PIPE_FORMAT_R16_UNORM:
>> + *stype = TGSI_RETURN_TYPE_UNORM;
>> + return "layout(r16) ";
>> + case PIPE_FORMAT_R16_SNORM:
>> + *stype = TGSI_RETURN_TYPE_SNORM;
>> + return "layout(r16_snorm) ";
>> + case PIPE_FORMAT_R16_UINT:
>> + *stype = TGSI_RETURN_TYPE_UINT;
>> + return "layout(r16ui) ";
>> + case PIPE_FORMAT_R16_SINT:
>> + *stype = TGSI_RETURN_TYPE_SINT;
>> + return "layout(r16i) ";
>> + case PIPE_FORMAT_R16_FLOAT:
>> + *stype = TGSI_RETURN_TYPE_FLOAT;
>> + return "layout(r16f) ";
>> + case PIPE_FORMAT_R16G16_UNORM:
>> + *stype = TGSI_RETURN_TYPE_UNORM;
>> + return "layout(rg16) ";
>> + case PIPE_FORMAT_R16G16_SNORM:
>> + *stype = TGSI_RETURN_TYPE_SNORM;
>> + return "layout(rg16_snorm) ";
>> + case PIPE_FORMAT_R16G16_UINT:
>> + *stype = TGSI_RETURN_TYPE_UINT;
>> + return "layout(rg16ui) ";
>> + case PIPE_FORMAT_R16G16_SINT:
>> + *stype = TGSI_RETURN_TYPE_SINT;
>> + return "layout(rg16i) ";
>> + case PIPE_FORMAT_R16G16_FLOAT:
>> + *stype = TGSI_RETURN_TYPE_FLOAT;
>> + return "layout(rg16f) ";
>> + case PIPE_FORMAT_R16G16B16A16_UNORM:
>> + *stype = TGSI_RETURN_TYPE_UNORM;
>> + return "layout(rgba16) ";
>> + case PIPE_FORMAT_R16G16B16A16_SNORM:
>> + *stype = TGSI_RETURN_TYPE_SNORM;
>> + return "layout(rgba16_snorm) ";
>> + case PIPE_FORMAT_R16G16B16A16_FLOAT:
>> + *stype = TGSI_RETURN_TYPE_FLOAT;
>> + return "layout(rgba16f) ";
>> + case PIPE_FORMAT_R32_FLOAT:
>> + *stype = TGSI_RETURN_TYPE_FLOAT;
>> + return "layout(r32f) ";
>> + case PIPE_FORMAT_R32_UINT:
>> + *stype = TGSI_RETURN_TYPE_UINT;
>> + return "layout(r32ui) ";
>> + case PIPE_FORMAT_R32_SINT:
>> + *stype = TGSI_RETURN_TYPE_SINT;
>> + return "layout(r32i) ";
>> + case PIPE_FORMAT_R32G32_FLOAT:
>> + *stype = TGSI_RETURN_TYPE_FLOAT;
>> + return "layout(rg32f) ";
>> + case PIPE_FORMAT_R32G32_UINT:
>> + *stype = TGSI_RETURN_TYPE_UINT;
>> + return "layout(rg32ui) ";
>> + case PIPE_FORMAT_R32G32_SINT:
>> + *stype = TGSI_RETURN_TYPE_SINT;
>> + return "layout(rg32i) ";
>> + case PIPE_FORMAT_R32G32B32A32_FLOAT:
>> + *stype = TGSI_RETURN_TYPE_FLOAT;
>> + return "layout(rgba32f) ";
>> + case PIPE_FORMAT_R32G32B32A32_UINT:
>> + *stype = TGSI_RETURN_TYPE_UINT;
>> + return "layout(rgba32ui) ";
>> + case PIPE_FORMAT_R16G16B16A16_UINT:
>> + *stype = TGSI_RETURN_TYPE_UINT;
>> + return "layout(rgba16ui) ";
>> + case PIPE_FORMAT_R16G16B16A16_SINT:
>> + *stype = TGSI_RETURN_TYPE_SINT;
>> + return "layout(rgba16i) ";
>> + case PIPE_FORMAT_R32G32B32A32_SINT:
>> + *stype = TGSI_RETURN_TYPE_SINT;
>> + return "layout(rgba32i) ";
>> + case PIPE_FORMAT_NONE:
>> + *stype = TGSI_RETURN_TYPE_UNORM;
>> + return "";
>> + default:
>> + *stype = TGSI_RETURN_TYPE_UNORM;
>> + fprintf(stderr, "illegal format %d\n", virgl_format);
>> + return "";
>> + }
>> +}
> Wouldn't it be better to add this information to the format table in
> vrend_format.c?
I don't think it buys us anything at this time, the shader strings
are pretty much set in stone by the extension, and they don't
really line up with some GL names in all cases I don't think.
>> >images_used_mask); j++) {
>> + if (!(ctx->images_used_mask & (1 << j)))
>> + break;
>> + if (!memcmp(&ctx->images[i].decl, &ctx->images[j].decl,
>> sizeof(ctx->images[j].decl)) &&
>> + ctx->images[i].image_return == ctx-
>> >images[j].image_return) {
>> + mask &= ~(1 << j);
>> + array_size++;
>> + }
>> + }
> Just to confirm: The intention is to add for each unique image an array
> of the size corresponding to the number this image declaration is
> repeated? (This is probably the point were I would start to write unit
> tests to see whether the TGSI produces the GLSL code I really want.)
> After staring at the code long enough I think I understand it now, but
> adding some comment would go along way ;)
I've rewritten the image array support, so it passes the tests I'll repost the
two patches that if affect, but yes you are correct about this code.
Dave.
More information about the virglrenderer-devel
mailing list