This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: RFA: Fix PR 20413
Roger Sayle wrote:
On Mon, 11 Apr 2005, Joern RENNECKE wrote:
2005-04-11 J"orn Rennecke <joern.rennecke@st.com>
Stephen Clarke <stevec@superh.com>
* cfgrtl.c (redirect_edge_and_branch): Use Pmode instead of
VOIDmode for LABEL_REF.
* final.c (shorten_branches): Likewise.
* jump.c (mark_all_labels, redirect_exp_1): Likewise.
* loop.c (reg_dead_after_loop): Likewise.
* varasm.c (decode_addr_const): Likewise.
Unfortunately PR rtl-optimization/20413 doesn't give a testcase to
backup it's claims of being an ICE-on-valid or even it's missed
optimization status.
Try gcc.dg/20050321-1.c in the sh64-elf branch for ice-on-valid:
/mnt/scratch/nightly/sh-elf-4_1-work/sh64-elf/gcc/xgcc
-B/mnt/scratch/nightly/sh-elf-4_1-work/sh64-elf/gcc/
/mnt/scratch/nightly/sh-elf-4_1-work/srcw/gcc/testsuite/gcc.dg/20050321-1.c
-O2 -fmodulo-sched -DSTACK_SIZE=16384 -S -isystem
/mnt/scratch/nightly/sh-elf-4_1-work/sh64-elf/sh64-elf/./newlib/targ-include
-isystem /mnt/scratch/nightly/sh-elf-4_1-work/srcw/newlib/libc/include
-o 20050321-1.s (timeout = 300)
/mnt/scratch/nightly/sh-elf-4_1-work/srcw/gcc/testsuite/gcc.dg/20050321-1.c:
In function 'foo':^M
/mnt/scratch/nightly/sh-elf-4_1-work/srcw/gcc/testsuite/gcc.dg/20050321-1.c:26:
error: unable to find a register to spill in class 'TARGET_REGS'^M
/mnt/scratch/nightly/sh-elf-4_1-work/srcw/gcc/testsuite/gcc.dg/20050321-1.c:26:
error: this is the insn:^M
(jump_insn:HI 47 46 121 2 (set (pc)^M
(if_then_else (ne (reg:SI 1 r1 [orig:176 D.1198 ] [176])^M
(reg:SI 2 r2 [orig:175 a1.3 ] [175]))^M
(label_ref 43)^M
(pc))) 211 {*beq_media_i32} (insn_list:REG_DEP_ANTI 46
(insn_list:REG_DEP_TRUE 45 (nil)))^M
(expr_list:REG_DEAD (reg:SI 1 r1 [orig:176 D.1198 ] [176])^M
(expr_list:REG_BR_PROB (const_int 8900 [0x22c4])^M
(nil))))^M
Without these patterns in sh.md:
;; ??? Workaround for PR rtl-optimization/20413
(define_split
[(set (pc)
(if_then_else (match_operator 3 "comparison_operator"
[(match_operand 1 "arith_reg_or_0_operand" "")
(match_operand 2 "arith_operand" "")])
(match_operand 0 "target_operand" "")
(pc)))]
"TARGET_SHMEDIA && GET_MODE (operands[0]) == VOIDmode
&& recog_memoized (insn)"
[(set (pc) (if_then_else (match_dup 3) (match_dup 0) (pc)))]
"PUT_MODE (operands[0], Pmode);")
(define_split
[(set (pc) (match_operand 0 "target_operand" ""))]
"TARGET_SHMEDIA && GET_MODE (operands[0]) == VOIDmode
&& recog_memoized (insn)"
[(set (pc) (match_dup 0))]
"PUT_MODE (operands[0], Pmode);")
The toolchain won't even build, because it ICEs while building
the libraries.
I have come cross a missed-optimization when I investigated why a
cross-jumping bug was not triggered. Most of the LABEL_REFs are
generated in Pmode, but some are generated in VOIDmode. Even if
label inside is the same, the LABEL_REFs won't match if the mode
doesn't match.
To come back to the ICE problem above: a problem of the sh64-elf port
in cvs mainline is that it creates paradoxical subregs for SHmedia32 when
generating calls. This is related to PR target/20695. To fix this, the
call, jump and branch patterns must accept a SImode operand for SHmedia32,
and a DImode operand in SHmedia64 - i.e. consistently Pmode.
However, you can't specify Pmode for an operand in an instruction pattern,
since it is not a constant; you have to specify VOIDmode instead and
make the
predicate check the mode.
This is all fine and well when your LABEL_REFs and SYMBOL_REFs are
Pmode, but if they are VOIDmode, reload doesn't know what mode it should
use when reloading constants into target registers - and hence the
reload failure.
The reason why this is important is that the
current RTL documentation in rtl.texi implies that the mode of a
LABEL_REF should always be VOIDmode:
Compare:
@findex symbol_ref
@item (symbol_ref:@var{mode} @var{symbol})
Represents the value of an assembler label for data.
@findex label_ref
@item (label_ref @var{label})
Represents the value of an assembler label for code.
Where the description of symbol_ref includes an explicit mode, and
the description of label_ref doesn't. Even if it were decided that
LABEL_REFs should always be Pmode, the documentation would need to
be updated to reflect that.
Do you want me to prepare a patch for that?
The distinction is that symbol refs may refer to different types
of symbol, local/global, exported, static, different sections, etc...
which may require that they are accessed in different modes. Labels
on the other hand are local to the current function and would always
be accessed with the same mode through control flow instructions.
Actually, there are nonlocal labels that can be accessed from a nested
function. In principle we could make some machine-specific attributes to
make the nested function a different mode internally but have an interface
that is compatible with the enclosing function.
However, far more likely is that there are bugs/misunderstandings
elsewhere in the compiler that are checking the mode of a LABEL_REF.
Just a wild guess, but I suspect one of these places is the SH
back-end?
The reload problem is not a misunderstanding, but genuinely missing
information. If neither the operand in the pattern nor the constant have a
mode, what mode is reload supposed to use for push_reload?
Without a testcase, or a pointer to the source that expects modes
on LABEL_REFs, it's impossible to say which is at fault and whether
it would be reasonable to change the long time definition of the
RTX code (potentially breaking other targets that may rely on the
documented behaviour).
This code in find_reloads uses the mode of the operand:
/* Now record reloads for all the operands that need them. */
...
else if (goal_alternative_matched[i] == -1)
{
operand_reloadnum[i]
= push_reload ((modified[i] != RELOAD_WRITE
? recog_data.operand[i] : 0),
(modified[i] != RELOAD_READ
? recog_data.operand[i] : 0),
(modified[i] != RELOAD_WRITE
? recog_data.operand_loc[i] : 0),
(modified[i] != RELOAD_READ
? recog_data.operand_loc[i] : 0),
(enum reg_class) goal_alternative[i],
(modified[i] == RELOAD_WRITE
? VOIDmode : operand_mode[i]),
(modified[i] == RELOAD_READ
? VOIDmode : operand_mode[i]),
(insn_code_number < 0 ? 0
:
insn_data[insn_code_number].operand[i].strict_low),
0, i, operand_type[i]);
Finally, I agree completely with Giovanni Bajo's comments that
whatever the outcome, we should consider removing the mode argument
from gen_rtx_LABEL_REF, if appropriate, to prevent these misunderstandings
in the future.
what should explow.c:convert_memory_address do with LABEL_REFs then?
Note: that this blocks merging of the sh-elf-4_1-branch, which has
had this potentially incorrect patch applied, until the issue is
resolved.
Actually, it hasn't had this patch applied.