Thursday, October 26, 2006

Software maintenance is dull. No one wants to do it, people would much rather be writing new code - or at least that's how the line usually goes. Most programmers think that software maintenance is either a drudge task or fixing bugs in code that others write. it can really be a time for an engineer to go back and rethink code that's written. After you've fixed the 3rd or 10th or 200th bug in a section of code you often end up with something that resembles spaghetti (from combine):

/* Don't eliminate a store in the stack pointer. */ if (dest == stack_pointer_rtx /* Don't combine with an insn that sets a register to itself if it has a REG_EQUAL note. This may be part of a REG_NO_CONFLICT sequence. */ || (rtx_equal_p (src, dest) && find_reg_note (insn, REG_EQUAL, NULL_RTX)) /* Can't merge an ASM_OPERANDS. */ || GET_CODE (src) == ASM_OPERANDS /* Can't merge a function call. */ || GET_CODE (src) == CALL /* Don't eliminate a function call argument. */ || (CALL_P (i3) && (find_reg_fusage (i3, USE, dest) || (REG_P (dest) && REGNO (dest) < FIRST_PSEUDO_REGISTER && global_regs[REGNO (dest)]))) /* Don't substitute into an incremented register. */ || FIND_REG_INC_NOTE (i3, dest) || (succ && FIND_REG_INC_NOTE (succ, dest)) /* Don't substitute into a non-local goto, this confuses CFG. */ || (JUMP_P (i3) && find_reg_note (i3, REG_NON_LOCAL_GOTO, NULL_RTX))#if 0 /* Don't combine the end of a libcall into anything. */ /* ??? This gives worse code, and appears to be unnecessary, since no pass after flow uses REG_LIBCALL/REG_RETVAL notes. Local-alloc does use REG_RETVAL notes for noconflict blocks, but other code here makes sure that those insns don't disappear. */ || find_reg_note (insn, REG_RETVAL, NULL_RTX)#endif /* Make sure that DEST is not used after SUCC but before I3. */ || (succ && ! all_adjacent && reg_used_between_p (dest, succ, i3)) /* Make sure that the value that is to be substituted for the register does not use any registers whose values alter in between. However, If the insns are adjacent, a use can't cross a set even though we think it might (this can happen for a sequence of insns each setting the same destination; last_set of that register might point to a NOTE). If INSN has a REG_EQUIV note, the register is always equivalent to the memory so the substitution is valid even if there are intervening stores. Also, don't move a volatile asm or UNSPEC_VOLATILE across any other insns. */ || (! all_adjacent && (((!MEM_P (src) || ! find_reg_note (insn, REG_EQUIV, src)) && use_crosses_set_p (src, INSN_CUID (insn))) || (GET_CODE (src) == ASM_OPERANDS && MEM_VOLATILE_P (src)) || GET_CODE (src) == UNSPEC_VOLATILE)) /* If there is a REG_NO_CONFLICT note for DEST in I3 or SUCC, we get better register allocation by not doing the combine. */ || find_reg_note (i3, REG_NO_CONFLICT, dest) || (succ && find_reg_note (succ, REG_NO_CONFLICT, dest)) /* Don't combine across a CALL_INSN, because that would possibly change whether the life span of some REGs crosses calls or not, and it is a pain to update that information. Exception: if source is a constant, moving it later can't hurt. Accept that special case, because it helps -fforce-addr a lot. */ || (INSN_CUID (insn) < last_call_cuid && ! CONSTANT_P (src))) return 0;

"When you throw away code and start from scratch, you are throwing away all that knowledge. All those collected bug fixes. Years of programming work."

Now, this code is ugly. It should probably be rethought and refactored a bit, but not rewritten completely. There's another area in that file that's a single function that is more than a thousand lines long. Should that be rethought? Probably. Deleted and rewritten from scratch? Unlikely.

This is how you turn dull software maintenance tasks into exciting new times: you periodically go back and rethink code, it's part of maintenance too and it gets you doing "new" code while doing the necessary bug fixing that got you into that code in the first place. A good rule of thumb is that every time you fix a bug you should try to clean up the surrounding code in some way so that the next person that goes through has an easier time than you did - and you don't lose the benefit of all of the years of bug fixing in that area.