Menu
▾
▴
#353 Wrong optimization in postreload pass. (+fix)
I have found an code optimization issue with gcc for msp430. Currently I use v4.6.3 with the LTS patch but the bug is probably in all variants (or actually in the non-arch specific part).
The issue is that my code is miscompiled because a mov instruction is removed which is required:
.loc 4 54 0
push.b #40
.LCFI13:
mov #llo(23438215), r12
mov #lhi(23438215), r13
mov.b #1, r14
mov r1, r15 <--------- This one is removed in postreload
add #556, r15
.LCFI14:
call #_ZN13CurrentSensorC1Ehmh
The reason is that the pushqi1 insn uses "pre_modify" and move2add_note_store() in postreload.c does not handle it. So I propose the following patch (inspired by gcc PR/47008):
diff -uNr gcc-4.6.3.orig/gcc/postreload.c gcc-4.6.3/gcc/postreload.c
--- gcc-4.6.3.orig/gcc/postreload.c 2011-02-11 21:52:55.000000000 +0100
+++ gcc-4.6.3/gcc/postreload.c 2013-05-09 14:21:39.963390084 +0200
@@ -2091,7 +2091,8 @@
{
dst = XEXP (dst, 0);
if (GET_CODE (dst) == PRE_INC || GET_CODE (dst) == POST_INC
- || GET_CODE (dst) == PRE_DEC || GET_CODE (dst) == POST_DEC)
+ || GET_CODE (dst) == PRE_DEC || GET_CODE (dst) == POST_DEC
+ || GET_CODE (dst) == PRE_MODIFY || GET_CODE (dst) == POST_MODIFY)
reg_set_luid[REGNO (XEXP (dst, 0))] = 0;
return;
}
Does this make sense? Dunno if this should be fixed upstream even if it is not yet triggered by any of the officially supported architectures.
At a quick glance, it makes sense, and is worth filing as an upstream "obvious" bug given the existing code is not consistent with the similar test in reload_combine_note_store. It'll probably be ignored, but should still be reported.
If you have a reproducing test case I'll consider it for any future patches to mspgcc, but since the Red Hat rewrite is being merged upstream it's not likely there will be more mspgcc patches.
Ok, I created an upstream bug report: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57229
The rewrite seems to different so it might not trigger the bug. So if it is unlikely that you are going to create any new mspgcc patches then I skip the pain of creating a minimal testcase. I already spent too much time in debugging this as I'm not really a compiler respective gcc guru. But at least I learned a lot so it was not a real waste of time. :-)