From: Maarten B. <sou...@ds...> - 2008-03-22 09:47:30
|
Yes, that might be redundant. But it was the original statement. It was good enough for genPlus but not for genAssign. Also all checks for AOP_SIZE<GPTRSIZE seem redundant to me, because it can only be >=GPTRSIZE if it is a GPTR which was covered above by opIsGptr(left or right). Moreover !sameRegs looks too strict to me, I think it could be !sameByte. But as I see it these are all optimizations not needed for fixing the bug at hand and may introduce a risk. So I vote to update this again after the release and go safe for now by only fixing the bug as proposed. Maarten > ... after rethinking: isn't the first condition after "if (opIsGptr > (IC_RESULT (ic)))" redundant? > If I'm not wrong, it would be enough: > > ... > > if (opIsGptr (IC_RESULT (ic))) > { > if (IC_LEFT (ic) && AOP_SIZE (IC_LEFT (ic)) < GPTRSIZE && > !sameRegs (AOP (IC_RESULT (ic)), AOP (IC_LEFT (ic)))) > { > char buffer[10]; > SNPRINTF (buffer, sizeof(buffer), "#0x%02x", > pointerTypeToGPByte (pointerCode (getSpec > (operandType (IC_LEFT (ic)))), NULL, NULL)); > aopPut (IC_RESULT (ic), buffer, GPTRSIZE - 1); > return; > } > if (IC_RIGHT (ic) && AOP_SIZE (IC_RIGHT (ic)) < GPTRSIZE && > !sameRegs (AOP (IC_RESULT (ic)), AOP (IC_RIGHT (ic)))) > { > char buffer[10]; > SNPRINTF (buffer, sizeof(buffer), "#0x%02x", > pointerTypeToGPByte (pointerCode (getSpec > (operandType (IC_RIGHT (ic)))), NULL, NULL)); > aopPut (IC_RESULT (ic), buffer, GPTRSIZE - 1); > return; > } > } > > ... > > Borut > > > Borut Razem wrote: > > Approved. > > > > Just a small optimization: define buffer[10] only once after > > if (opIsGptr (IC_RESULT (ic))) > > { > > > > Borut > > > > Maarten Brock wrote: > > > >> Borut, > >> > >> Unfortunately further investigation indicates my patch was not good > >> enough to also fix bug 1839299. I need to take > >> adjustArithmeticResult() just a step further. Attached is the whole > >> function for replacement in src/mcs51/gen.c > >> > >> Once again I ask for approval. > >> > >> Maarten > >> > >> > >> > >> > >>> Thank you both, > >>> > >>> It is commited in revision #5111. > >>> > >>> Maarten |