On Tuesday, September 23, 2014 01:25:55 PM Matt Turner wrote: > On Tue, Sep 23, 2014 at 1:10 PM, Jason Ekstrand <ja...@jlekstrand.net> wrote: > > On Thu, Aug 28, 2014 at 8:10 PM, Matt Turner <matts...@gmail.com> wrote: > >> + int jump = brw_inst_imm_d(brw, insn); > >> + int jump_compacted = jump / sizeof(brw_compact_inst); > >> + int jump_uncompacted = jump / sizeof(brw_inst); > >> + > >> + target_old_ip = this_old_ip + jump_uncompacted; > >> + target_compacted_count = compacted_counts[target_old_ip]; > >> + jump_compacted -= (target_compacted_count - > >> this_compacted_count); > >> + brw_inst_set_imm_ud(brw, insn, jump_compacted * > >> + sizeof(brw_compact_inst)); > > > > > > Any reason why you're reading it as a signed value and then writing it back > > in unsigned? > > IIRC brw_inst_set_imm_d() is broken. It's been long enough that I've > forgotten the exact details, but _ud sets the exact bits you pass it, > while _d does something wrong.
static inline void
brw_inst_set_imm_d(const struct brw_context *brw,
brw_inst *insn, int value)
{
(void) brw;
return brw_inst_set_bits(insn, 127, 96, value);
}
The final parameter of brw_inst_set_bits is a uint64_t, so it looks like the
"int" value will get sign-extended to a 64-bit value, then treated as unsigned,
which is now too wide to fit in bits 127:96. IOW, it will never work for
negative values...while, ironically, brw_inst_set_imm_ud() will. (The int
would be converted to unsigned...still 32-bit...then promoted from unsigned to
uint64_t, which doesn't sign extend.)
That said, fixing it would be trivial:
return brw_inst_set_bits(insn, 127, 96, (unsigned) value);
We should either fix it or delete it. Either is fine by me.
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
