[Mesa-dev] [PATCH v2 08/13] st/glsl_to_tgsi: disable on-the-fly peephole for 64-bit operations

[Nicolai Hähnle]

From: Nicolai Hähnle <nicolai.haehnle at amd.com>

This optimization is incorrect with 64-bit operations, because the
channel-splitting logic in emit_asm ends up being applied twice to
the source operands.

A lucky coincidence of how the writemask test works resulted in this
optimization basically never being applied anyway. As far as I can tell,
the only case where it would (incorrectly) have been applied is something
like

    dvec2 d;
    float x = (float)d.y;

which nobody seems to have ever done. But the moral equivalent does occur
in one of the component layout piglit test.

Cc: mesa-stable at lists.freedesktop.org
---
 src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index 444f5f7..66ce24c 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -248,20 +248,21 @@ class glsl_to_tgsi_instruction : public exec_node {
 public:
    DECLARE_RALLOC_CXX_OPERATORS(glsl_to_tgsi_instruction)
 
    unsigned op;
    st_dst_reg dst[2];
    st_src_reg src[4];
    /** Pointer to the ir source this tree came from for debugging */
    ir_instruction *ir;
    GLboolean cond_update;
    bool saturate;
+   bool is_64bit_expanded;
    st_src_reg sampler; /**< sampler register */
    int sampler_base;
    int sampler_array_size; /**< 1-based size of sampler array, 1 if not array */
    int tex_target; /**< One of TEXTURE_*_INDEX */
    glsl_base_type tex_type;
    GLboolean tex_shadow;
    unsigned image_format;
 
    st_src_reg tex_offsets[MAX_GLSL_TEXTURE_OFFSET];
    unsigned tex_offset_num_offset;
@@ -663,20 +664,21 @@ glsl_to_tgsi_visitor::emit_asm(ir_instruction *ir, unsigned op,
    assert(num_reladdr == 0);
 
    inst->op = op;
    inst->info = tgsi_get_opcode_info(op);
    inst->dst[0] = dst;
    inst->dst[1] = dst1;
    inst->src[0] = src0;
    inst->src[1] = src1;
    inst->src[2] = src2;
    inst->src[3] = src3;
+   inst->is_64bit_expanded = false;
    inst->ir = ir;
    inst->dead_mask = 0;
    /* default to float, for paths where this is not initialized
     * (since 0==UINT which is likely wrong):
     */
    inst->tex_type = GLSL_TYPE_FLOAT;
 
    inst->function = NULL;
 
    /* Update indirect addressing status used by TGSI */
@@ -783,20 +785,21 @@ glsl_to_tgsi_visitor::emit_asm(ir_instruction *ir, unsigned op,
          if (dinst == NULL) {
             dinst = inst;
          } else {
             /* create a new instructions for subsequent attempts */
             dinst = new(mem_ctx) glsl_to_tgsi_instruction();
             *dinst = *inst;
             dinst->next = NULL;
             dinst->prev = NULL;
          }
          this->instructions.push_tail(dinst);
+         dinst->is_64bit_expanded = true;
 
          /* modify the destination if we are splitting */
          for (j = 0; j < 2; j++) {
             if (dst_is_64bit[j]) {
                dinst->dst[j].writemask = (i & 1) ? WRITEMASK_ZW : WRITEMASK_XY;
                dinst->dst[j].index = initial_dst_idx[j];
                if (i > 1) {
                   if (dinst->op == TGSI_OPCODE_STORE) {
                      dinst->src[0] = addr;
                   } else {
@@ -2901,20 +2904,21 @@ glsl_to_tgsi_visitor::visit(ir_assignment *ir)
    assert(r.file != PROGRAM_UNDEFINED);
 
    if (ir->condition) {
       const bool switch_order = this->process_move_condition(ir->condition);
       st_src_reg condition = this->result;
 
       emit_block_mov(ir, ir->lhs->type, &l, &r, &condition, switch_order);
    } else if (ir->rhs->as_expression() &&
               this->instructions.get_tail() &&
               ir->rhs == ((glsl_to_tgsi_instruction *)this->instructions.get_tail())->ir &&
+              !((glsl_to_tgsi_instruction *)this->instructions.get_tail())->is_64bit_expanded &&
               type_size(ir->lhs->type) == 1 &&
               l.writemask == ((glsl_to_tgsi_instruction *)this->instructions.get_tail())->dst[0].writemask) {
       /* To avoid emitting an extra MOV when assigning an expression to a
        * variable, emit the last instruction of the expression again, but
        * replace the destination register with the target of the assignment.
        * Dead code elimination will remove the original instruction.
        */
       glsl_to_tgsi_instruction *inst, *new_inst;
       inst = (glsl_to_tgsi_instruction *)this->instructions.get_tail();
       new_inst = emit_asm(ir, inst->op, l, inst->src[0], inst->src[1], inst->src[2], inst->src[3]);
-- 
2.7.4