On 26/08/18 08:24, Luc Van Oostenryck wrote: > 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. OK ATB, Ramsay Jones
