On Sat, Aug 25, 2018 at 09:54:32PM +0100, Ramsay Jones wrote: > On 25/08/18 16:43, Luc Van Oostenryck wrote: > > diff --git a/cse.c b/cse.c > > index 1395a8af3..22dfd4ba5 100644 > > --- a/cse.c > > +++ b/cse.c > > @@ -76,6 +76,7 @@ void cse_collect(struct instruction *insn) > > /* Unary */ > > case OP_NOT: case OP_NEG: > > case OP_FNEG: > > + case OP_SYMADDR: > > hash += hashval(insn->src1); > > s/src1/src/ ? (We fall through from the binary cases above, so > this compares to the use of the 'src2' field there. However, I > would even prefer divorcing the binary and unary cases, use 'src' > here and _add_ a 'break' and 'src1' there! ;-) ). But then exactly the same code would need to be duplicated for src1 and for src, here and in liveness.c too. In fact I have some patches that do exactly the opposite ... I think that the most important is to be well aware that src1, src, cond, _cond, addr, phi_src, base, func (and the now gone symbol, and phi_var in the new SSA code) denote exactly the same field and exist more to have a readable definition of struct instruction. It's of course, much better, in general, to use the/a good name for each instructions but in some performance critical parts like here using this aliasing property is, IMO, not a bad thing, certainly when the code show clearly what's happening. -- Luc
