The C code below extern __attribute__((pure)) int pf (int *); extern int bar; int foo (void) { return pf (&bar);} is wrongly compiled with -fPIC on sh64-elf. The output doesn't any references to the symbol bar: .section .text..SHmedia32,"ax" .align 2 .global _foo .type _foo, @function _foo: addi.l r15, -24, r15 st.l r15, 12, r18 st.l r15, 8, r14 st.l r15, 4, r12 movi (((datalabel _GLOBAL_OFFSET_TABLE_-(.LPCS0-.)) >> 16) & 65535), r12 shori ((datalabel _GLOBAL_OFFSET_TABLE_-(.LPCS0-.)) & 65535), r12 .LPCS0: ptrel/u r12, tr0 gettr tr0, r12 add.l r15, r63, r14 ld.l r2, 0, r2 movi (((_pf@GOTPLT) >> 16) & 65535), r1 shori ((_pf@GOTPLT) & 65535), r1 ldx.l r1, r12, r1 ptabs r1, tr0 blink tr0, r18 add.l r14, r63, r15 ld.l r15, 4, r12 ld.l r15, 8, r14 ld.l r15, 12, r18 addi.l r15, 24, r15 ptabs r18, tr0 blink tr0, r63 .size _foo, .-_foo I confirmed that 3.4/4.0 behaves same and 3.3.6 compiler produces the sane code. So this is a regression from 3.3 to 3.4/4.0/4.1.
I've traced what's going on: http://gcc.gnu.org/ml/gcc-patches/2005-03/msg00525.html It includes a patch for comment.
(In reply to comment #1) > I've traced what's going on: > http://gcc.gnu.org/ml/gcc-patches/2005-03/msg00525.html > It includes a patch for comment. > The sh64 port is partcularily exposed in emit_libcall_block because it uses paradoxical subregs to set up the offset part of the PIC address, but I think your patch makes sense. Except that this is only the tip of the iceberg. The problem was introduced in a maga-patch to remove RTX_UNCHANGING_P; there are a number of other places that need to be fixed up similarily. 2004-08-18 Richard Henderson <rth@redhat.com> * rtl.h (MEM_READONLY_P): Replace RTX_UNCHANGING_P. * alias.c (true_dependence): Update to match new semantics. (canon_true_dependence, write_dependence_p): Likewise. (anti_dependence, output_dependence): Update write_dependence_p args. (unchanging_anti_dependence): Remove. * calls.c (purge_mem_unchanging_flag): Remove. (fixup_tail_calls): Don't call it. (expand_call): Don't add unchanging memory to function usage. * expr.c (emit_block_move_via_libcall): Likewise. (clear_storage_via_libcall): Don't clobber RTX_UNCHANGING_P mems. (get_subtarget): Don't use RTX_UNCHANGING_P. (expand_assignment, store_constructor, expand_expr_real_1): Likewise. (do_tablejump): Set MEM_READONLY_P, not RTX_UNCHANGING_P. * combine.c (get_last_value_validate): Use MEM_READONLY_P. * cse.c (insert): Don't use RTX_UNCHANGING_P. (cse_insn, canon_hash): Use MEM_READONLY_P. * emit-rtl.c (set_mem_attributes_minus_bitpos): Use MEM_READONLY_P instead of RTX_UNCHANGING_P. * explow.c (maybe_set_unchanging): Remove. * expr.h (maybe_set_unchanging): Remove. * flow.c (insn_dead_p, mark_used_regs): Use anti_dependence. * function.c (assign_stack_temp_for_type): Don't use RTX_UNCHANGING_P. (assign_parm_setup_reg, expand_function_start): Likewise. * integrate.c (copy_rtx_and_substitute): Likewise. * ra-rewrite.c (emit_colors): Likewise. * regmove.c (copy_src_to_dest, regmove_optimize): Likewise. (fixup_match_1): Likewise. * reload1.c (reload, alter_reg): Likewise. * local-alloc.c (validate_equiv_mem): Check MEM_READONLY_P, not RTX_UNCHANGING_P. (equiv_init_varies_p): Likewise. * loop-invariant.c (check_maybe_invariant): Likewise. * resource.c (mark_referenced_resources, mark_set_resources): Likewise. * loop.c (note_addr_stored): Likewise. (prescan_loop): Likewise. Don't check function usage for clobbered unchanging memory. * rtlanal.c (rtx_unstable_p): Check MEM_READONLY_P, not RTX_UNCHANGING_P. (rtx_varies_p, modified_between_p, modified_in_p): Likewise. * varasm.c (force_const_mem): Likewise. * stmt.c (expand_decl): Don't set RTX_UNCHANGING_P. * web.c (entry_register): Likewise. * tree-gimple.h (get_base_address): Move decl ... * tree.h: ... here. * doc/rtl.texi (MEM_READONLY_P): Replace RTX_UNCHANGING_P. * config/alpha/alpha.c (alpha_set_memflags_1): Rewrite to be called via for_each_rtx. Copy MEM_SCALAR_P, MEM_NOTRAP_P too. (alpha_set_memflags): Update to match. * config/darwin.c (machopic_indirect_data_reference): Set MEM_READONLY_P instead of RTX_UNCHANGING_P. (machopic_indirect_call_target): Likewise. (machopic_legitimize_pic_address): Likewise. * config/arm/arm.c (legitimize_pic_address, arm_gen_load_multiple, arm_gen_store_multiple, arm_gen_movmemqi): Likewise. * config/arm/arm.md (load_multiple, store_multiple): Likewise. * config/frv/frv.md (symGOT2reg): Likewise. * config/i386/i386.c (legitimize_pic_address, legitimize_tls_address, ix86_split_to_parts): Likewise. * config/ia64/ia64.c (ia64_expand_tls_address): Likewise. * config/ia64/ia64.md (load_fptr): Likewise. * config/m32r/m32r.c (m32r_legitimize_pic_address): Likewise. * config/m68k/m68k.c (legitimize_pic_address): Likewise. * config/mcore/mcore.c (block_move_sequence): Likewise. * config/mn10300/mn10300.md (symGOT2reg): Likewise. * config/pa/pa.c (legitimize_pic_address): Likewise. * config/rs6000/rs6000.c (rs6000_legitimize_tls_address): Likewise. (rs6000_emit_move): Likewise. * config/s390/s390.c (legitimize_pic_address): Likewise. (legitimize_tls_address): Likewise. * config/s390/s390.md (casesi): Likewise. * config/sh/sh.c (prepare_move_operands, sh_reorg): Likewise. * config/sh/sh.md (symGOT2reg): Likewise. * config/sparc/sparc.c (legitimize_pic_address): Likewise. * config/v850/v850.md (casesi): Likewise. * config/ia64/ia64.c (gen_thread_pointer): Don't set RTX_UNCHANGING_P. * config/iq2000/iq2000.c (save_restore_insns): Likewise. * config/mips/mips.c (mips_restore_gp): Likewise. (mips_save_restore_reg, mips16_gp_pseudo_reg): Likewise. * config/sh/sh.c (sh_reorg): Likewise.
I'm taking a look at the uses of MEM_READONLY_P, though I'm not sure which one has the similar issue. It looks that modified_in_p has the same problem with modified_between_p. BTW, it's better to change the component of this PR, isn't it? rtl-optimization?
CC'ing RTH since he's the one who removed RTX_UNCHANGING.
(In reply to comment #3) > I'm taking a look at the uses of MEM_READONLY_P, though I'm not > sure which one has the similar issue. It looks that modified_in_p > has the same problem with modified_between_p. Yes, that's also the one I spotted first. But it's simple to find more: grep tells us alias.c has some MEM_READONLY_P. Bingo, the first hit is another instance: true_dependence does: if (MEM_READONLY_P (x)) return 0; without checking if x contains a MEM that is dependent (possible if the target supports memory-indirect addressing, e.g. m68020)
I'll agree that modified_{in,between}_p need to check the address for changes, since that controls the ultimate value that is accessed. I do not agree that alias.c needs to check the address for changes. In that case we're trying to determine if assigning a value to one memory reference can possibly change the value of another memory reference. Which is false for *all* read-only memory.
Subject: Re: [3.4/4.0/4.1 Regression] Wrong code generation for the argument of the pure function in PIC rth at gcc dot gnu dot org wrote: >------- Additional Comments From rth at gcc dot gnu dot org 2005-03-10 13:57 ------- >I'll agree that modified_{in,between}_p need to check the address for changes, >since that controls the ultimate value that is accessed. > >I do not agree that alias.c needs to check the address for changes. In that >case we're trying to determine if assigning a value to one memory reference >can possibly change the value of another memory reference. Which is false for >*all* read-only memory. > > > After looking a bit more at true_dependence and friends, I think you are right; the callers have checked or will be checking the address. And apart from the dependency functions in alias.c, I could find only modified_{in,between}_p using MEM_READONLY_P to indicate that the entire MEM doesn't change.
Subject: Bug 20331 CVSROOT: /cvs/gcc Module name: gcc Changes by: kkojima@gcc.gnu.org 2005-03-11 03:14:45 Modified files: gcc : ChangeLog rtlanal.c Log message: PR rtl-optimization/20331 * rtlanal.c (modified_between_p): Check its address first for MEM. (modified_in_p): Likewise. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.7780&r2=2.7781 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/rtlanal.c.diff?cvsroot=gcc&r1=1.211&r2=1.212
Is this fixed now that a patch has been applied?
On 4.1, yes.
It's marked as a 4.0 regression. Do you plan on applying the patch there? If not, we should simply close the bug.
It causes the failure only for sh64 target and can be found only at 4.x java runtime for that target on which 4.0 jave doesn't work without many other patches. So there would be no reason to apply it to 4.0 and it'd be enough to close this PR.
Fine.