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: diff --git a/cse.c b/cse.c index 511ac88..739c255 100644 --- a/cse.c +++ b/cse.c @@ -207,18 +207,16 @@ static int insn_compare(const void *_i1, const void *_i2) case_binops: if (i1->src2 != i2->src2) return i1->src2 < i2->src2 ? -1 : 1; - /* Fall through to unops */ - - /* Unary */ - case OP_NOT: case OP_NEG: - case OP_FNEG: if (i1->src1 != i2->src1) return i1->src1 < i2->src1 ? -1 : 1; break; + /* Unary */ + case OP_NOT: case OP_NEG: + case OP_FNEG: case OP_SYMADDR: - if (i1->symbol != i2->symbol) - return i1->symbol < i2->symbol ? -1 : 1; + if (i1->src != i2->src) + return i1->src < i2->src ? -1 : 1; break; case OP_SETVAL: ATB, Ramsay Jones
