On Sun, Aug 26, 2018 at 12:29:59AM +0100, Ramsay Jones wrote: > > > On 25/08/18 23:31, Luc Van Oostenryck wrote: > > 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 ... > > Hmm, so you think a single additional if statement is too > much duplication? Just to be sure, what I was suggesting > would look like so: Yes, it's what I understood. I don't care much but in truth I prefer the reversed patch. Also, I consider as an error to treat these src/src1 here differently than in the hashing. -- Luc
