On 05/07/2015 07:59 AM, Richard Sandiford wrote:
Jeff Law <l...@redhat.com> writes:
On 05/07/2015 05:37 AM, Richard Sandiford wrote:
One problem with the automatically-generated gen_rtx_FOO () macros
is that they always have a mode parameter, even for codes like SET
where the mode should always be VOIDmode. This inevitably leads to
cases where a caller accidentally passes something other than VOIDmode.
E.g. when expanding an SImode move, the temptation is to make everything
SImode, even the SETs. This in turn can cause two instructions to appear
different simply because their SETs have different modes, even though the
SET_DEST and SET_SRC are identical.
E.g. for gcc/testsuite/g++.dg/torture/pr34651.C on lm32-elf we have
the following before jump2:
(jump_insn 42 191 43 5 (set (pc)
(if_then_else (eq:SI (reg:SI 13 r13 [orig:43 inHotKey$4+-3 ] [43])
(const_int 0 [0]))
(label_ref 53)
(pc))) gcc/testsuite/g++.dg/torture/pr34651.C:22 22 {*beq}
(expr_list:REG_DEAD (reg:SI 13 r13 [orig:43 inHotKey$4+-3 ] [43])
(int_list:REG_BR_PROB 5000 (nil)))
-> 53)
(note 43 42 48 6 [bb 6] NOTE_INSN_BASIC_BLOCK)
(insn 48 43 47 6 (set (reg:SI 2 r2)
(mem/u/c:SI (reg:SI 1 r1) [4 S4 A32]))
gcc/testsuite/g++.dg/torture/pr34651.C:22 7 {movsi_insn}
(expr_list:REG_DEAD (reg:SI 1 r1)
(nil)))
[...]
(code_label 53 169 54 7 6 "" [1 uses])
(note 54 53 12 7 [bb 7] NOTE_INSN_BASIC_BLOCK)
(insn 12 54 57 7 (set:SI (reg/f:SI 2 r2 [orig:46 D.2050 ] [46])
(mem/u/c:SI (reg:SI 1 r1) [4 S4 A32]))
gcc/testsuite/g++.dg/torture/pr34651.C:22 7 {movsi_insn}
(expr_list:REG_DEAD (reg:SI 1 r1)
(expr_list:REG_EQUAL (symbol_ref/f:SI ("*.LC3") [flags 0x2] <var_decl #
*.LC3>)
(nil))))
where insns 12 and 48 are identical except for the :SI on the SET.
This difference prevents us from merging the heads of the two blocks;
after removing it we replace the two loads with a single load before
the branch.
This patch removes the mode argument from gen_rtx_SET and updates
all callers. I used a script to (try to) make sure that all callers
really had been caught. I also built one target per CPU just in case.
There were some changes in gcc.dg, g++.dg and gcc.c-torture assembly
code for c6x-elf, lm32-elf and v850-elf, but all of them seemed to be
code improvements from removing duplicated instructions. (Other ports
also passed spurious modes but apparently not in a way that affects
the tests I'd tried.) Also tested on x86_64-linux-gnu. OK to install?
BTW, I've split the patch up into two, the last bit being a mechanical
removal of modes. (I did it by hand though to try to keep things
properly formatted.)
Thanks,
Richard
gcc/
* rtl.h (always_void_p): New function.
* gengenrtl.c (always_void_p(: Likewise.
Typo in the ChangeLog.
non-mechanical part didn't seem to be attached. I don't expect any
problems there, but a quick looksie seems appropriate.
Bah. Sorry about that.
It's unfortunate that we have two copies of the test -- one on codes
and one on strings -- but that's how everything else in gengenrtl.c
is done. Maybe a later clean-up.
Looks good to me.
I wonder if there's others with similar characteristics... The toplevel
objects come to mind, but ultimately we don't want them to be RTXs at
all IMHO.
Anyway, it's a good cleanup unto itself and clearly folks have gotten it
wrong in the past based on your testing results. Thanks for taking care
of it.
Jeff
