| Message ID | 201007280944.52575.ebotcazou@adacore.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Jul 28, 2010 at 12:44 AM, Eric Botcazou <ebotcazou@adacore.com> wrote: > This is the bootstrap failure on SPARC64/Linux introduced by the fix for PR > middle-end/44790, which was a bootstrap failure on IA-64/HP-UX introduced by > the mem-ref2 merge. > > The fix introduced a non-canonical expansion for MEM_REF via POINTER_PLUS_EXPR > which bypasses checks for valid addresses in the SPARC back-end: > > name = MEM[(struct > exp_ch3__make_predefined_primitive_specs__B_99__stream_op_tss_names___PAD > *)D.14526_1156 + 4294967296B]; > > is expanded into > > sethi %hi(stream_op_tss_names.6060+4294967296), %l2 > or %l2, %lo(stream_op_tss_names.6060+4294967296), %l2 > > which overflows since sethi is a 32-bit operator. > > This can very likely happen for other back-ends as well so I think that the > best approach is to fix PR middle-end/44790 more canonically. The problem > was that: > > op0 = expand_expr (base, NULL_RTX, address_mode, EXPAND_NORMAL); > > assumed that op0 was generated in address_mode; this isn't guaranteed so an > explicit address conversion is required: > > op0 = convert_memory_address_addr_space (address_mode, op0, as); > > > Bootstrapped/regtested on SPARC64/Linux, applied on the mainline as obvious. > > > 2010-07-28 Eric Botcazou <ebotcazou@adacore.com> > > PR middle-end/44790 > PR middle-end/44993 > * expr.c (expand_expr_real_1) <MEM_REF>: Revert latest change. Make > sure the base has address_mode before adding the offset. > This patch caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45670
> This patch caused: > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45670 No, it didn't, it cannot cause a code size regression from GCC 4.5 since it reverted an incorrect change made on trunk only. Start from 161906 please.
On Tue, Sep 14, 2010 at 10:51 PM, Eric Botcazou <ebotcazou@adacore.com> wrote: > No, it didn't, it cannot cause a code size regression from GCC 4.5 since it > reverted an incorrect change made on trunk only. Start from 161906 please. By the way I noticed iv-opts differences between 4.3 and 4.6. So you might want to consider IV-opts changes first. -- Pinski
On Tue, Sep 14, 2010 at 10:27 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Wed, Jul 28, 2010 at 12:44 AM, Eric Botcazou <ebotcazou@adacore.com> wrote: >> This is the bootstrap failure on SPARC64/Linux introduced by the fix for PR >> middle-end/44790, which was a bootstrap failure on IA-64/HP-UX introduced by >> the mem-ref2 merge. >> >> The fix introduced a non-canonical expansion for MEM_REF via POINTER_PLUS_EXPR >> which bypasses checks for valid addresses in the SPARC back-end: >> >> name = MEM[(struct >> exp_ch3__make_predefined_primitive_specs__B_99__stream_op_tss_names___PAD >> *)D.14526_1156 + 4294967296B]; >> >> is expanded into >> >> sethi %hi(stream_op_tss_names.6060+4294967296), %l2 >> or %l2, %lo(stream_op_tss_names.6060+4294967296), %l2 >> >> which overflows since sethi is a 32-bit operator. >> >> This can very likely happen for other back-ends as well so I think that the >> best approach is to fix PR middle-end/44790 more canonically. The problem >> was that: >> >> op0 = expand_expr (base, NULL_RTX, address_mode, EXPAND_NORMAL); >> >> assumed that op0 was generated in address_mode; this isn't guaranteed so an >> explicit address conversion is required: >> >> op0 = convert_memory_address_addr_space (address_mode, op0, as); >> >> >> Bootstrapped/regtested on SPARC64/Linux, applied on the mainline as obvious. >> >> >> 2010-07-28 Eric Botcazou <ebotcazou@adacore.com> >> >> PR middle-end/44790 >> PR middle-end/44993 >> * expr.c (expand_expr_real_1) <MEM_REF>: Revert latest change. Make >> sure the base has address_mode before adding the offset. >> > > This patch caused: > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45670 > This also caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46252
> This also caused: > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46252 Again, no, it didn't, it cannot cause a regression from GCC 4.5 since it reverted an incorrect change made on trunk only.
Index: expr.c =================================================================== --- expr.c (revision 162566) +++ expr.c (working copy) @@ -8730,11 +8730,14 @@ expand_expr_real_1 (tree exp, rtx target base = build2 (BIT_AND_EXPR, TREE_TYPE (base), gimple_assign_rhs1 (def_stmt), gimple_assign_rhs2 (def_stmt)); + op0 = expand_expr (base, NULL_RTX, VOIDmode, EXPAND_NORMAL); + op0 = convert_memory_address_addr_space (address_mode, op0, as); if (!integer_zerop (TREE_OPERAND (exp, 1))) - base = build2 (POINTER_PLUS_EXPR, TREE_TYPE (base), - base, double_int_to_tree (sizetype, - mem_ref_offset (exp))); - op0 = expand_expr (base, NULL_RTX, address_mode, EXPAND_SUM); + { + rtx off + = immed_double_int_const (mem_ref_offset (exp), address_mode); + op0 = simplify_gen_binary (PLUS, address_mode, op0, off); + } op0 = memory_address_addr_space (mode, op0, as); temp = gen_rtx_MEM (mode, op0); set_mem_attributes (temp, exp, 0);
This is the bootstrap failure on SPARC64/Linux introduced by the fix for PR middle-end/44790, which was a bootstrap failure on IA-64/HP-UX introduced by the mem-ref2 merge. The fix introduced a non-canonical expansion for MEM_REF via POINTER_PLUS_EXPR which bypasses checks for valid addresses in the SPARC back-end: name = MEM[(struct exp_ch3__make_predefined_primitive_specs__B_99__stream_op_tss_names___PAD *)D.14526_1156 + 4294967296B]; is expanded into sethi %hi(stream_op_tss_names.6060+4294967296), %l2 or %l2, %lo(stream_op_tss_names.6060+4294967296), %l2 which overflows since sethi is a 32-bit operator. This can very likely happen for other back-ends as well so I think that the best approach is to fix PR middle-end/44790 more canonically. The problem was that: op0 = expand_expr (base, NULL_RTX, address_mode, EXPAND_NORMAL); assumed that op0 was generated in address_mode; this isn't guaranteed so an explicit address conversion is required: op0 = convert_memory_address_addr_space (address_mode, op0, as); Bootstrapped/regtested on SPARC64/Linux, applied on the mainline as obvious. 2010-07-28 Eric Botcazou <ebotcazou@adacore.com> PR middle-end/44790 PR middle-end/44993 * expr.c (expand_expr_real_1) <MEM_REF>: Revert latest change. Make sure the base has address_mode before adding the offset.