[Mesa-dev] [PATCH 2/2] gallium: Desambiguate TGSI_OPCODE_IF.
Jose Fonseca
jfonseca at vmware.com
Sun Apr 14 15:33:55 PDT 2013
----- Original Message -----
> Am 14.04.2013 10:12, schrieb jfonseca at vmware.com:
> > From: José Fonseca <jfonseca at vmware.com>
> >
> > TGSI_OPCODE_IF condition had two possible interpretations:
> >
> > - src.x != 0.0f
> >
> > - Mesa statetracker when PIPE_SHADER_CAP_INTEGERS was false either for
> > vertex and fragment shaders
> > - gallivm/llvmpipe
> > - postprocess
> > - vl state tracker
> > - vega state tracker
> > - most old drivers
> > - old internal state trackers
> > - many graw examples
> >
> > - src.x != 0U
> >
> > - Mesa statetracker when PIPE_SHADER_CAP_INTEGERS was true for both
> > vertex and fragment shaders
> > - tgsi_exec/softpipe
> > - r600
> > - radeonsi
> > - nv50
> >
> > And drivers that use draw module also were a mess (because Mesa would
> > emit float IFs, but draw module supports native integers so it would
> > interpret IF arg as integers...)
> >
> > This sort of works if the source argument is limited to float +0.0f or
> > +1.0f, integer 0, but would fail if source is float -0.0f, or integer in
> > the float NaN range. It could also fail if source is integer 1, and
> > hardware flushes denormalized numbers to zero.
> >
> > But with this change there are now two opcodes, IF and UIF, with clear
> > meaning.
> >
> > Drivers that do not support native integers do not need to worry about
> > UIF. However, for backwards compatibility with old state trackers and
> > examples, it is advisable that native integer capable drivers also
> > support the float IF opcode.
> >
> > I tried to implement this for r600 and radeonsi based on the surrounding
> > code. I couldn't do this for nouveau, so I just shunted IF/UIF
> > together, which matches the current behavior.
> > ---
> > src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c | 1 +
> > src/gallium/auxiliary/gallivm/lp_bld_tgsi_aos.c | 1 +
> > src/gallium/auxiliary/gallivm/lp_bld_tgsi_info.c | 1 +
> > src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c | 19 ++++++++-
> > src/gallium/auxiliary/tgsi/tgsi_dump.c | 2 +
> > src/gallium/auxiliary/tgsi/tgsi_exec.c | 22 +++++++++++
> > src/gallium/auxiliary/tgsi/tgsi_info.c | 2 +-
> > src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h | 1 +
> > src/gallium/docs/source/tgsi.rst | 21 ++++++++--
> > .../drivers/nv50/codegen/nv50_ir_from_tgsi.cpp | 6 +++
> > src/gallium/drivers/r600/r600_shader.c | 21 +++++++---
> > .../drivers/radeon/radeon_setup_tgsi_llvm.c | 41
> > ++++++++++++++++----
> > src/gallium/include/pipe/p_shader_tokens.h | 2 +-
> > src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 8 +++-
> > src/mesa/state_tracker/st_mesa_to_tgsi.c | 12 +++++-
> > 15 files changed, 137 insertions(+), 23 deletions(-)
> >
> > diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c
> > b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c
> > index c71c1f1..e1c362b 100644
> > --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c
> > +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c
> > @@ -868,6 +868,7 @@ lp_set_default_actions(struct lp_build_tgsi_context *
> > bld_base)
> > bld_base->op_actions[TGSI_OPCODE_COS].fetch_args =
> > scalar_unary_fetch_args;
> > bld_base->op_actions[TGSI_OPCODE_EX2].fetch_args =
> > scalar_unary_fetch_args;
> > bld_base->op_actions[TGSI_OPCODE_IF].fetch_args =
> > scalar_unary_fetch_args;
> > + bld_base->op_actions[TGSI_OPCODE_UIF].fetch_args =
> > scalar_unary_fetch_args;
> > bld_base->op_actions[TGSI_OPCODE_KIL].fetch_args = kil_fetch_args;
> > bld_base->op_actions[TGSI_OPCODE_KILP].fetch_args = kilp_fetch_args;
> > bld_base->op_actions[TGSI_OPCODE_RCP].fetch_args =
> > scalar_unary_fetch_args;
> > diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_aos.c
> > b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_aos.c
> > index 98bce0e..223184d 100644
> > --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_aos.c
> > +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_aos.c
> > @@ -837,6 +837,7 @@ lp_emit_instruction_aos(
> > return FALSE;
> >
> > case TGSI_OPCODE_IF:
> > + case TGSI_OPCODE_UIF:
> > return FALSE;
> >
> > case TGSI_OPCODE_BGNLOOP:
> > diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_info.c
> > b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_info.c
> > index 3c79abf..b00aa09 100644
> > --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_info.c
> > +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_info.c
> > @@ -389,6 +389,7 @@ analyse_instruction(struct analysis_context *ctx,
> >
> > switch (inst->Instruction.Opcode) {
> > case TGSI_OPCODE_IF:
> > + case TGSI_OPCODE_UIF:
> > case TGSI_OPCODE_ELSE:
> > case TGSI_OPCODE_ENDIF:
> > case TGSI_OPCODE_BGNLOOP:
> Could you also add it to tgsi_opcode_infer_src_type?
Good point. Will do. tgsi_opcode_infer_src_type has a default catch all for float, so I missed that.
> > diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
> > b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
> > index 239530d..362a1de 100644
> > --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
> > +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
> > @@ -1732,7 +1732,8 @@ near_end_of_shader(struct lp_build_tgsi_soa_context
> > *bld,
> > opcode == TGSI_OPCODE_CAL ||
> > opcode == TGSI_OPCODE_CALLNZ ||
> > opcode == TGSI_OPCODE_IF ||
> > - opcode == TGSI_OPCODE_BGNLOOP ||
> > + opcode == TGSI_OPCODE_UIF ||
> > + opcode == TGSI_OPCODE_BGNLOOP ||
> > opcode == TGSI_OPCODE_SWITCH)
> > return FALSE;
> > }
> > @@ -2395,6 +2396,21 @@ if_emit(
> > }
> >
> > static void
> > +uif_emit(
> > + const struct lp_build_tgsi_action * action,
> > + struct lp_build_tgsi_context * bld_base,
> > + struct lp_build_emit_data * emit_data)
> > +{
> > + LLVMValueRef tmp;
> > + struct lp_build_tgsi_soa_context * bld = lp_soa_context(bld_base);
> > + struct lp_build_context *uint_bld = &bld_base->uint_bld;
> > +
> > + tmp = lp_build_cmp(uint_bld, PIPE_FUNC_NOTEQUAL,
> > + emit_data->args[0], uint_bld->zero);
> > + lp_exec_mask_cond_push(&bld->exec_mask, tmp);
> > +}
> > +
> > +static void
> > bgnloop_emit(
> > const struct lp_build_tgsi_action * action,
> > struct lp_build_tgsi_context * bld_base,
> > @@ -2742,6 +2758,7 @@ lp_build_tgsi_soa(struct gallivm_state *gallivm,
> > bld.bld_base.op_actions[TGSI_OPCODE_ENDLOOP].emit = endloop_emit;
> > bld.bld_base.op_actions[TGSI_OPCODE_ENDSUB].emit = endsub_emit;
> > bld.bld_base.op_actions[TGSI_OPCODE_IF].emit = if_emit;
> > + bld.bld_base.op_actions[TGSI_OPCODE_UIF].emit = uif_emit;
> > bld.bld_base.op_actions[TGSI_OPCODE_KIL].emit = kil_emit;
> > bld.bld_base.op_actions[TGSI_OPCODE_KILP].emit = kilp_emit;
> > bld.bld_base.op_actions[TGSI_OPCODE_NRM].emit = nrm_emit;
> > diff --git a/src/gallium/auxiliary/tgsi/tgsi_dump.c
> > b/src/gallium/auxiliary/tgsi/tgsi_dump.c
> > index 365665f..77b75b1 100644
> > --- a/src/gallium/auxiliary/tgsi/tgsi_dump.c
> > +++ b/src/gallium/auxiliary/tgsi/tgsi_dump.c
> > @@ -589,6 +589,7 @@ iter_instruction(
> >
> > switch (inst->Instruction.Opcode) {
> > case TGSI_OPCODE_IF:
> > + case TGSI_OPCODE_UIF:
> > case TGSI_OPCODE_ELSE:
> > case TGSI_OPCODE_BGNLOOP:
> > case TGSI_OPCODE_ENDLOOP:
> > @@ -600,6 +601,7 @@ iter_instruction(
> >
> > /* update indentation */
> > if (inst->Instruction.Opcode == TGSI_OPCODE_IF ||
> > + inst->Instruction.Opcode == TGSI_OPCODE_UIF ||
> > inst->Instruction.Opcode == TGSI_OPCODE_ELSE ||
> > inst->Instruction.Opcode == TGSI_OPCODE_BGNLOOP) {
> > ctx->indentation += indent_spaces;
> > diff --git a/src/gallium/auxiliary/tgsi/tgsi_exec.c
> > b/src/gallium/auxiliary/tgsi/tgsi_exec.c
> > index 8579d8a..15eea1f 100644
> > --- a/src/gallium/auxiliary/tgsi/tgsi_exec.c
> > +++ b/src/gallium/auxiliary/tgsi/tgsi_exec.c
> > @@ -3982,6 +3982,28 @@ exec_instruction(
> > mach->CondStack[mach->CondStackTop++] = mach->CondMask;
> > FETCH( &r[0], 0, TGSI_CHAN_X );
> > /* update CondMask */
> > + if( ! r[0].f[0] ) {
> > + mach->CondMask &= ~0x1;
> > + }
> > + if( ! r[0].f[1] ) {
> > + mach->CondMask &= ~0x2;
> > + }
> > + if( ! r[0].f[2] ) {
> > + mach->CondMask &= ~0x4;
> > + }
> > + if( ! r[0].f[3] ) {
> > + mach->CondMask &= ~0x8;
> > + }
> > + UPDATE_EXEC_MASK(mach);
> > + /* Todo: If CondMask==0, jump to ELSE */
> > + break;
> > +
> > + case TGSI_OPCODE_UIF:
> > + /* push CondMask */
> > + assert(mach->CondStackTop < TGSI_EXEC_MAX_COND_NESTING);
> > + mach->CondStack[mach->CondStackTop++] = mach->CondMask;
> > + FETCH( &r[0], 0, TGSI_CHAN_X );
> While this was there before, technically using FETCH is not entirely
> correct since it will fetch a float. Now since it goes into a union
> anyway this shouldn't really matter, but src modifiers would be handled
> wrong. I guess though modifiers aren't really allowed with that opcode.
I'll replace with IFETCH.
For the record, gallivm moves ints as floats all the time. I think that it is fair to assume that moving floats around does not change their bits. It might not be explicit promissed anyway, but just common sense: any compiler will just move bits.
> > + /* update CondMask */
> > if( ! r[0].u[0] ) {
> > mach->CondMask &= ~0x1;
> > }
> > diff --git a/src/gallium/auxiliary/tgsi/tgsi_info.c
> > b/src/gallium/auxiliary/tgsi/tgsi_info.c
> > index 716b16b..11ba0b6 100644
> > --- a/src/gallium/auxiliary/tgsi/tgsi_info.c
> > +++ b/src/gallium/auxiliary/tgsi/tgsi_info.c
> > @@ -112,7 +112,7 @@ static const struct tgsi_opcode_info
> > opcode_info[TGSI_OPCODE_LAST] =
> > { 1, 2, 1, 0, 0, 0, OTHR, "TXL", TGSI_OPCODE_TXL },
> > { 0, 0, 0, 0, 0, 0, NONE, "BRK", TGSI_OPCODE_BRK },
> > { 0, 1, 0, 1, 0, 1, NONE, "IF", TGSI_OPCODE_IF },
> > - { 1, 1, 0, 0, 0, 1, NONE, "", 75 }, /* removed */
> > + { 0, 1, 0, 1, 0, 1, NONE, "UIF", TGSI_OPCODE_UIF },
> > { 0, 1, 0, 0, 0, 1, NONE, "", 76 }, /* removed */
> > { 0, 0, 0, 1, 1, 1, NONE, "ELSE", TGSI_OPCODE_ELSE },
> > { 0, 0, 0, 0, 1, 0, NONE, "ENDIF", TGSI_OPCODE_ENDIF },
> > diff --git a/src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h
> > b/src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h
> > index b8519c6..a6bcbb0 100644
> > --- a/src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h
> > +++ b/src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h
> > @@ -128,6 +128,7 @@ OP12(DP2)
> > OP12_TEX(TXL)
> > OP00(BRK)
> > OP01_LBL(IF)
> > +OP01_LBL(UIF)
> > OP00_LBL(ELSE)
> > OP00(ENDIF)
> > OP01(PUSHA)
> > diff --git a/src/gallium/docs/source/tgsi.rst
> > b/src/gallium/docs/source/tgsi.rst
> > index 0002626..b7180f8 100644
> > --- a/src/gallium/docs/source/tgsi.rst
> > +++ b/src/gallium/docs/source/tgsi.rst
> > @@ -864,19 +864,32 @@ This instruction replicates its result.
> > TBD
> >
> >
> > -.. opcode:: IF - If
> > +.. opcode:: IF - Float If
> >
> > - TBD
> > + Start an IF ... ELSE .. ENDIF block. Condition evaluates to true if
> > +
> > + src0.x != 0.0
> > +
> > + where src0.x is interpreted as a floating point register.
> Maybe should say something wrt evaluation of NaNs? I know we haven't
> really established rules for comparisons etc. wrt NaNs but those
> bools-as-float make me cry. I guess it is no different though than other
> float opcodes, if we now really have a definition saying IF takes _any_
> float not just a bool-as-float which was loosely implied before.
I haven't though about NaNs but see no reason to address that immediately.
If you believe that bool-as-float was loosely implied before, then you can hold on to that faith: I haven't done anything that would results in different values being passed to IF opcode.
I documented the condition as "src0.x != 0.0f" because that's how all drivers implement it in practice. And there nothing special about that. Just like KIL's condition is "src.x < 0 || src.y < 0 || src.z < 0 || src.w < 0".
Personally I'd like gallium ecosystem to move away from implementing logic operations as floating operations. These should be reduced to relics of the past. My change is just one step towards that, but I believe that Mesa and other state trackers are still full of boolean logic.
> > +
> > +
> > +.. opcode:: UIF - Bitwise If
> > +
> > + Start an UIF ... ELSE .. ENDIF block. Condition evaluates to true if
> > +
> > + src0.x != 0
> > +
> > + where src0.x is interpreted as an integer register.
> >
> >
> > .. opcode:: ELSE - Else
> >
> > - TBD
> > + Starts an else block, after an IF or UIF statement.
> >
> >
> > .. opcode:: ENDIF - End If
> >
> > - TBD
> > + Ends an IF or UIF block.
> >
> >
> > .. opcode:: PUSHA - Push Address Register On Stack
> > diff --git a/src/gallium/drivers/nv50/codegen/nv50_ir_from_tgsi.cpp
> > b/src/gallium/drivers/nv50/codegen/nv50_ir_from_tgsi.cpp
> > index 6897691..054c75e 100644
> > --- a/src/gallium/drivers/nv50/codegen/nv50_ir_from_tgsi.cpp
> > +++ b/src/gallium/drivers/nv50/codegen/nv50_ir_from_tgsi.cpp
> > @@ -242,6 +242,7 @@ unsigned int Instruction::srcMask(unsigned int s) const
> > case TGSI_OPCODE_SCS:
> > return 0x1;
> > case TGSI_OPCODE_IF:
> > + case TGSI_OPCODE_UIF:
> > return 0x1;
> > case TGSI_OPCODE_LIT:
> > return 0xb;
> > @@ -2430,6 +2431,11 @@ Converter::handleInstruction(const struct
> > tgsi_full_instruction *insn)
> > mkOp1(op, TYPE_U32, NULL, src0)->fixed = 1;
> > break;
> > case TGSI_OPCODE_IF:
> > + /* XXX: fall-through into UIF, but this might lead to
> > + * incorrect behavior on state trackers and auxiliary
> > + * modules that emit float bool IFs regardless of
> > + * native integer support */
> > + case TGSI_OPCODE_UIF:
> > {
> > BasicBlock *ifBB = new BasicBlock(func);
> >
> > diff --git a/src/gallium/drivers/r600/r600_shader.c
> > b/src/gallium/drivers/r600/r600_shader.c
> > index 44b5ce5..6e8c7a2 100644
> > --- a/src/gallium/drivers/r600/r600_shader.c
> > +++ b/src/gallium/drivers/r600/r600_shader.c
> > @@ -5730,6 +5730,18 @@ static void break_loop_on_flag(struct
> > r600_shader_ctx *ctx, unsigned fc_sp)
> >
> > static int tgsi_if(struct r600_shader_ctx *ctx)
> > {
> > + emit_logic_pred(ctx, ALU_OP2_PRED_SETNE);
> > +
> > + r600_bytecode_add_cfinst(ctx->bc, CF_OP_JUMP);
> > +
> > + fc_pushlevel(ctx, FC_IF);
> > +
> > + callstack_push(ctx, FC_PUSH_VPM);
> > + return 0;
> > +}
> > +
> > +static int tgsi_uif(struct r600_shader_ctx *ctx)
> > +{
> > emit_logic_pred(ctx, ALU_OP2_PRED_SETNE_INT);
> >
> > r600_bytecode_add_cfinst(ctx->bc, CF_OP_JUMP);
> > @@ -5991,8 +6003,7 @@ static struct r600_shader_tgsi_instruction
> > r600_shader_tgsi_instruction[] = {
> > {TGSI_OPCODE_TXL, 0, FETCH_OP_SAMPLE_L, tgsi_tex},
> > {TGSI_OPCODE_BRK, 0, CF_OP_LOOP_BREAK, tgsi_loop_brk_cont},
> > {TGSI_OPCODE_IF, 0, ALU_OP0_NOP, tgsi_if},
> > - /* gap */
> > - {75, 0, ALU_OP0_NOP, tgsi_unsupported},
> > + {TGSI_OPCODE_UIF, 0, ALU_OP0_NOP, tgsi_uif},
> > {76, 0, ALU_OP0_NOP, tgsi_unsupported},
> > {TGSI_OPCODE_ELSE, 0, ALU_OP0_NOP, tgsi_else},
> > {TGSI_OPCODE_ENDIF, 0, ALU_OP0_NOP, tgsi_endif},
> > @@ -6185,8 +6196,7 @@ static struct r600_shader_tgsi_instruction
> > eg_shader_tgsi_instruction[] = {
> > {TGSI_OPCODE_TXL, 0, FETCH_OP_SAMPLE_L, tgsi_tex},
> > {TGSI_OPCODE_BRK, 0, CF_OP_LOOP_BREAK, tgsi_loop_brk_cont},
> > {TGSI_OPCODE_IF, 0, ALU_OP0_NOP, tgsi_if},
> > - /* gap */
> > - {75, 0, ALU_OP0_NOP, tgsi_unsupported},
> > + {TGSI_OPCODE_UIF, 0, ALU_OP0_NOP, tgsi_uif},
> > {76, 0, ALU_OP0_NOP, tgsi_unsupported},
> > {TGSI_OPCODE_ELSE, 0, ALU_OP0_NOP, tgsi_else},
> > {TGSI_OPCODE_ENDIF, 0, ALU_OP0_NOP, tgsi_endif},
> > @@ -6379,8 +6389,7 @@ static struct r600_shader_tgsi_instruction
> > cm_shader_tgsi_instruction[] = {
> > {TGSI_OPCODE_TXL, 0, FETCH_OP_SAMPLE_L, tgsi_tex},
> > {TGSI_OPCODE_BRK, 0, CF_OP_LOOP_BREAK, tgsi_loop_brk_cont},
> > {TGSI_OPCODE_IF, 0, ALU_OP0_NOP, tgsi_if},
> > - /* gap */
> > - {75, 0, ALU_OP0_NOP, tgsi_unsupported},
> > + {TGSI_OPCODE_UIF, 0, ALU_OP0_NOP, tgsi_uif},
> > {76, 0, ALU_OP0_NOP, tgsi_unsupported},
> > {TGSI_OPCODE_ELSE, 0, ALU_OP0_NOP, tgsi_else},
> > {TGSI_OPCODE_ENDIF, 0, ALU_OP0_NOP, tgsi_endif},
> > diff --git a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
> > b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
> > index 314c963..dfc7cbc 100644
> > --- a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
> > +++ b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
> > @@ -515,20 +515,16 @@ static void endloop_emit(
> > ctx->loop_depth--;
> > }
> >
> > -static void if_emit(
> > +static void if_cond_emit(
> > const struct lp_build_tgsi_action * action,
> > struct lp_build_tgsi_context * bld_base,
> > - struct lp_build_emit_data * emit_data)
> > + struct lp_build_emit_data * emit_data,
> > + LLVMValueRef cond)
> > {
> > struct radeon_llvm_context * ctx = radeon_llvm_context(bld_base);
> > struct gallivm_state * gallivm = bld_base->base.gallivm;
> > - LLVMValueRef cond;
> > LLVMBasicBlockRef if_block, else_block, endif_block;
> >
> > - cond = LLVMBuildICmp(gallivm->builder, LLVMIntNE,
> > - bitcast(bld_base, TGSI_TYPE_UNSIGNED, emit_data->args[0]),
> > - bld_base->int_bld.zero, "");
> > -
> > endif_block = LLVMAppendBasicBlockInContext(gallivm->context,
> > ctx->main_fn, "ENDIF");
> > if_block = LLVMInsertBasicBlockInContext(gallivm->context,
> > @@ -545,6 +541,36 @@ static void if_emit(
> > ctx->branch[ctx->branch_depth - 1].has_else = 0;
> > }
> >
> > +static void if_emit(
> > + const struct lp_build_tgsi_action * action,
> > + struct lp_build_tgsi_context * bld_base,
> > + struct lp_build_emit_data * emit_data)
> > +{
> > + struct gallivm_state * gallivm = bld_base->base.gallivm;
> > + LLVMValueRef cond;
> > +
> > + cond = LLVMBuildFCmp(gallivm->builder, LLVMRealUNE,
> > + emit_data->args[0],
> > + bld_base->base.zero, "");
> > +
> > + if_cond_emit(action, bld_base, emit_data, cond);
> > +}
> > +
> > +static void uif_emit(
> > + const struct lp_build_tgsi_action * action,
> > + struct lp_build_tgsi_context * bld_base,
> > + struct lp_build_emit_data * emit_data)
> > +{
> > + struct gallivm_state * gallivm = bld_base->base.gallivm;
> > + LLVMValueRef cond;
> > +
> > + cond = LLVMBuildICmp(gallivm->builder, LLVMIntNE,
> > + bitcast(bld_base, TGSI_TYPE_UNSIGNED, emit_data->args[0]),
> > + bld_base->int_bld.zero, "");
> > +
> > + if_cond_emit(action, bld_base, emit_data, cond);
> > +}
> > +
> > static void kil_emit(
> > const struct lp_build_tgsi_action * action,
> > struct lp_build_tgsi_context * bld_base,
> > @@ -1209,6 +1235,7 @@ void radeon_llvm_context_init(struct
> > radeon_llvm_context * ctx)
> > bld_base->op_actions[TGSI_OPCODE_IABS].intr_name = "llvm.AMDIL.abs.";
> > bld_base->op_actions[TGSI_OPCODE_IDIV].emit = emit_idiv;
> > bld_base->op_actions[TGSI_OPCODE_IF].emit = if_emit;
> > + bld_base->op_actions[TGSI_OPCODE_IF].emit = uif_emit;
> > bld_base->op_actions[TGSI_OPCODE_IMAX].emit = build_tgsi_intrinsic_nomem;
> > bld_base->op_actions[TGSI_OPCODE_IMAX].intr_name = "llvm.AMDGPU.imax";
> > bld_base->op_actions[TGSI_OPCODE_IMIN].emit = build_tgsi_intrinsic_nomem;
> > diff --git a/src/gallium/include/pipe/p_shader_tokens.h
> > b/src/gallium/include/pipe/p_shader_tokens.h
> > index 6fcd151..50de2d3 100644
> > --- a/src/gallium/include/pipe/p_shader_tokens.h
> > +++ b/src/gallium/include/pipe/p_shader_tokens.h
> > @@ -335,7 +335,7 @@ struct tgsi_property_data {
> > #define TGSI_OPCODE_TXL 72
> > #define TGSI_OPCODE_BRK 73
> > #define TGSI_OPCODE_IF 74
> > - /* gap */
> > +#define TGSI_OPCODE_UIF 75
> > #define TGSI_OPCODE_ELSE 77
> > #define TGSI_OPCODE_ENDIF 78
> > /* gap */
> > diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> > b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> > index 589c912..f2eb3e7 100644
> > --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> > +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> > @@ -2978,12 +2978,15 @@ glsl_to_tgsi_visitor::visit(ir_discard *ir)
> > void
> > glsl_to_tgsi_visitor::visit(ir_if *ir)
> > {
> > + unsigned if_opcode;
> > glsl_to_tgsi_instruction *if_inst;
> >
> > ir->condition->accept(this);
> > assert(this->result.file != PROGRAM_UNDEFINED);
> >
> > - if_inst = emit(ir->condition, TGSI_OPCODE_IF, undef_dst, this->result);
> > + if_opcode = native_integers ? TGSI_OPCODE_UIF : TGSI_OPCODE_IF;
> > +
> > + if_inst = emit(ir->condition, if_opcode, undef_dst, this->result);
> >
> > this->instructions.push_tail(if_inst);
> >
> > @@ -3462,6 +3465,7 @@ glsl_to_tgsi_visitor::copy_propagate(void)
> > break;
> >
> > case TGSI_OPCODE_IF:
> > + case TGSI_OPCODE_UIF:
> > ++level;
> > break;
> >
> > @@ -3664,6 +3668,7 @@
> > glsl_to_tgsi_visitor::eliminate_dead_code_advanced(void)
> > break;
> >
> > case TGSI_OPCODE_IF:
> > + case TGSI_OPCODE_UIF:
> > ++level;
> > /* fallthrough to default case to mark the condition as read */
> >
> > @@ -4389,6 +4394,7 @@ compile_tgsi_instruction(struct st_translate *t,
> > case TGSI_OPCODE_ELSE:
> > case TGSI_OPCODE_ENDLOOP:
> > case TGSI_OPCODE_IF:
> > + case TGSI_OPCODE_UIF:
> > assert(num_dst == 0);
> > ureg_label_insn(ureg,
> > inst->op,
> > diff --git a/src/mesa/state_tracker/st_mesa_to_tgsi.c
> > b/src/mesa/state_tracker/st_mesa_to_tgsi.c
> > index 5471a9a..27d8bea 100644
> > --- a/src/mesa/state_tracker/st_mesa_to_tgsi.c
> > +++ b/src/mesa/state_tracker/st_mesa_to_tgsi.c
> > @@ -665,6 +665,7 @@ translate_opcode( unsigned op )
> >
> > static void
> > compile_instruction(
> > + struct gl_context *ctx,
> > struct st_translate *t,
> > const struct prog_instruction *inst,
> > boolean clamp_dst_color_output)
> > @@ -697,7 +698,6 @@ compile_instruction(
> > case OPCODE_CAL:
> > case OPCODE_ELSE:
> > case OPCODE_ENDLOOP:
> > - case OPCODE_IF:
> > debug_assert(num_dst == 0);
> > ureg_label_insn( ureg,
> > translate_opcode( inst->Opcode ),
> > @@ -705,6 +705,14 @@ compile_instruction(
> > get_label( t, inst->BranchTarget ));
> > return;
> >
> > + case OPCODE_IF:
> > + debug_assert(num_dst == 0);
> > + ureg_label_insn( ureg,
> > + ctx->Const.NativeIntegers ? TGSI_OPCODE_UIF :
> > TGSI_OPCODE_IF,
> > + src, num_src,
> > + get_label( t, inst->BranchTarget ));
> > + return;
> > +
> > case OPCODE_TEX:
> > case OPCODE_TXB:
> > case OPCODE_TXD:
> > @@ -1229,7 +1237,7 @@ st_translate_mesa_program(
> > */
> > for (i = 0; i < program->NumInstructions; i++) {
> > set_insn_start( t, ureg_get_instruction_number( ureg ));
> > - compile_instruction( t, &program->Instructions[i], clamp_color );
> > + compile_instruction( ctx, t, &program->Instructions[i], clamp_color
> > );
> > }
> >
> > /* Fix up all emitted labels:
> >
>
> Oh and I think that really should be "Disambiguate" in the commit title.
> Otherwise looks great, let's get rid of that mess.
>
> Reviewed-by: Roland Scheidegger <sroland at vmware.com>
Thanks.
Jose
More information about the mesa-dev
mailing list