| Message ID | CAFULd4ZPsqrq1O6kaEtQEkYyucdXNTN-vHyhdDWzXEzPjmdvgg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
With the patch bootstrapping x86_64-apple-darwin14 was broken due to a typo:
../../work/gcc/config/i386/i386.c: In function 'void ix86_expand_move(machine_mode, rtx_def**)':
../../work/gcc/config/i386/i386.c:17296:32: error: expected ',' or ';' before ')' token
? op0 : gen_reg_rtx (Pmode));
^
After removing the extra right parenthesis, I am now at stage 2.
Dominique
> Le 19 avr. 2015 à 20:35, Uros Bizjak <ubizjak@gmail.com> a écrit :
>
> Hello!
>
> Attached patch removes reload_in_progress checks for x86 (LRA enabled)
> target. AFAICS, reload_in_progress is never set during the
> compilation, a watchpoint on this variable didn't trigger for a couple
> of complex compilations.
>
> 2015-04-19 Uros Bizjak <ubizjak@gmail.com>
>
> * config/i386/i386.c (set_pic_reg_ever_live): Remove.
> (legitimize_pic_address): Do not call set_pic_reg_ever_live.
> (legitimize_tls_address): Ditto.
> (ix86_expand_move): Ditto.
> (ix86_expand_binary_operator): Remove reload_in_progress checks.
> (ix86_expand_unary_operator): Ditto.
> * config/i386/predicates.md (index_register_operand): Ditto.
>
> Patch was bootstrapped on x86_64-linux-gnu and regression tested for
> x86_64-linux-gnu, i686-linux-gnu with and w/o -fpic.
>
> The patch also changes Darwin specific code that can't be tested
> properly on linux. Instead of leaving a reload_in_progress_check
> theere, I'd ask someone to bootstrap and regression test the patch on
> Darwin target.
>
> I'll wait for the Darwin regression test results (and possible
> comments) before committing the patch.
>
> Uros.
> <p.diff.txt>
After having fixed the typo, regtesting went without regression. Dominique > Le 19 avr. 2015 à 20:35, Uros Bizjak <ubizjak@gmail.com> a écrit : > > Hello! > > Attached patch removes reload_in_progress checks for x86 (LRA enabled) > target. AFAICS, reload_in_progress is never set during the > compilation, a watchpoint on this variable didn't trigger for a couple > of complex compilations. > > 2015-04-19 Uros Bizjak <ubizjak@gmail.com> > > * config/i386/i386.c (set_pic_reg_ever_live): Remove. > (legitimize_pic_address): Do not call set_pic_reg_ever_live. > (legitimize_tls_address): Ditto. > (ix86_expand_move): Ditto. > (ix86_expand_binary_operator): Remove reload_in_progress checks. > (ix86_expand_unary_operator): Ditto. > * config/i386/predicates.md (index_register_operand): Ditto. > > Patch was bootstrapped on x86_64-linux-gnu and regression tested for > x86_64-linux-gnu, i686-linux-gnu with and w/o -fpic. > > The patch also changes Darwin specific code that can't be tested > properly on linux. Instead of leaving a reload_in_progress_check > theere, I'd ask someone to bootstrap and regression test the patch on > Darwin target. > > I'll wait for the Darwin regression test results (and possible > comments) before committing the patch. > > Uros. > <p.diff.txt>
On 20 Apr 2015, at 10:47, Dominique d'Humières wrote: > After having fixed the typo, regtesting went without regression. I have done a bootstrap on i686-darwin10 with the amended patch - slow machine, so testing still in progress (but looks OK so far), NOTE: that there some references to reload_in_progress in config/darwin.c pic code shared between powerpc and x86 darwin implementations. I will do a follow-up patch to make those assert if triggered on x86 (AFAIK, they still need to be present for powerpc, at present). Iain > > Dominique > >> Le 19 avr. 2015 à 20:35, Uros Bizjak <ubizjak@gmail.com> a écrit : >> >> Hello! >> >> Attached patch removes reload_in_progress checks for x86 (LRA enabled) >> target. AFAICS, reload_in_progress is never set during the >> compilation, a watchpoint on this variable didn't trigger for a couple >> of complex compilations. >> >> 2015-04-19 Uros Bizjak <ubizjak@gmail.com> >> >> * config/i386/i386.c (set_pic_reg_ever_live): Remove. >> (legitimize_pic_address): Do not call set_pic_reg_ever_live. >> (legitimize_tls_address): Ditto. >> (ix86_expand_move): Ditto. >> (ix86_expand_binary_operator): Remove reload_in_progress checks. >> (ix86_expand_unary_operator): Ditto. >> * config/i386/predicates.md (index_register_operand): Ditto. >> >> Patch was bootstrapped on x86_64-linux-gnu and regression tested for >> x86_64-linux-gnu, i686-linux-gnu with and w/o -fpic. >> >> The patch also changes Darwin specific code that can't be tested >> properly on linux. Instead of leaving a reload_in_progress_check >> theere, I'd ask someone to bootstrap and regression test the patch on >> Darwin target. >> >> I'll wait for the Darwin regression test results (and possible >> comments) before committing the patch. >> >> Uros. >> <p.diff.txt> >
On Mon, Apr 20, 2015 at 12:00 PM, Iain Sandoe <iain@codesourcery.com> wrote: >> After having fixed the typo, regtesting went without regression. > > I have done a bootstrap on i686-darwin10 with the amended patch - slow machine, so testing still in progress (but looks OK so far), > > NOTE: that there some references to reload_in_progress in config/darwin.c pic code shared between powerpc and x86 darwin implementations. I will do a follow-up patch to make those assert if triggered on x86 (AFAIK, they still need to be present for powerpc, at present). Probably a better way is to include "targetm.lra_p ()" into the check. Uros.
On Apr 20, 2015, at 3:16 AM, Uros Bizjak <ubizjak@gmail.com> wrote: > On Mon, Apr 20, 2015 at 12:00 PM, Iain Sandoe <iain@codesourcery.com> wrote: > >>> After having fixed the typo, regtesting went without regression. >> >> I have done a bootstrap on i686-darwin10 with the amended patch - slow machine, so testing still in progress (but looks OK so far), >> >> NOTE: that there some references to reload_in_progress in config/darwin.c pic code shared between powerpc and x86 darwin implementations. I will do a follow-up patch to make those assert if triggered on x86 (AFAIK, they still need to be present for powerpc, at present). > > Probably a better way is to include "targetm.lra_p ()" into the check. Only if you discount asking the nice rs6000/powerpc people to lra the entire port, and by that, I mean, remove the non-lra code. :-)
On Mon, Apr 20, 2015 at 12:00 PM, Iain Sandoe <iain@codesourcery.com> wrote: > > On 20 Apr 2015, at 10:47, Dominique d'Humières wrote: > >> After having fixed the typo, regtesting went without regression. > > I have done a bootstrap on i686-darwin10 with the amended patch - slow machine, so testing still in progress (but looks OK so far), > > NOTE: that there some references to reload_in_progress in config/darwin.c pic code shared between powerpc and x86 darwin implementations. I will do a follow-up patch to make those assert if triggered on x86 (AFAIK, they still need to be present for powerpc, at present). Thanks! I have committed the fixed patch to the SVN as r222246. Uros.
Index: config/i386/i386.c =================================================================== --- config/i386/i386.c (revision 222194) +++ config/i386/i386.c (working copy) @@ -13576,15 +13576,6 @@ ix86_GOT_alias_set (void) return set; } -/* Set regs_ever_live for PIC base address register - to true if required. */ -static void -set_pic_reg_ever_live () -{ - if (reload_in_progress) - df_set_regs_ever_live (REGNO (pic_offset_table_rtx), true); -} - /* Return a legitimate reference for ORIG (an address) using the register REG. If REG is 0, a new pseudo is generated. @@ -13635,7 +13626,6 @@ legitimize_pic_address (rtx orig, rtx reg) /* This symbol may be referenced via a displacement from the PIC base address (@GOTOFF). */ - set_pic_reg_ever_live (); if (GET_CODE (addr) == CONST) addr = XEXP (addr, 0); if (GET_CODE (addr) == PLUS) @@ -13667,7 +13657,6 @@ legitimize_pic_address (rtx orig, rtx reg) /* This symbol may be referenced via a displacement from the PIC base address (@GOTOFF). */ - set_pic_reg_ever_live (); if (GET_CODE (addr) == CONST) addr = XEXP (addr, 0); if (GET_CODE (addr) == PLUS) @@ -13728,7 +13717,6 @@ legitimize_pic_address (rtx orig, rtx reg) /* This symbol must be referenced via a load from the Global Offset Table (@GOT). */ - set_pic_reg_ever_live (); new_rtx = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, addr), UNSPEC_GOT); new_rtx = gen_rtx_CONST (Pmode, new_rtx); if (TARGET_64BIT) @@ -13780,7 +13768,6 @@ legitimize_pic_address (rtx orig, rtx reg) { if (!TARGET_64BIT) { - set_pic_reg_ever_live (); new_rtx = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, op0), UNSPEC_GOTOFF); new_rtx = gen_rtx_PLUS (Pmode, new_rtx, op1); @@ -14082,7 +14069,6 @@ legitimize_tls_address (rtx x, enum tls_model mode } else if (flag_pic) { - set_pic_reg_ever_live (); pic = pic_offset_table_rtx; type = TARGET_ANY_GNU_TLS ? UNSPEC_GOTNTPOFF : UNSPEC_GOTTPOFF; } @@ -17306,10 +17292,8 @@ ix86_expand_move (machine_mode mode, rtx operands[ /* dynamic-no-pic */ if (MACHOPIC_INDIRECT) { - rtx temp = ((reload_in_progress - || ((op0 && REG_P (op0)) - && mode == Pmode)) - ? op0 : gen_reg_rtx (Pmode)); + rtx temp = (op0 && REG_P (op0) && mode == Pmode) + ? op0 : gen_reg_rtx (Pmode)); op1 = machopic_indirect_data_reference (op1, temp); if (MACHOPIC_PURE) op1 = machopic_legitimize_pic_address (op1, mode, @@ -17957,17 +17941,11 @@ ix86_expand_binary_operator (enum rtx_code code, m /* Emit the instruction. */ op = gen_rtx_SET (VOIDmode, dst, gen_rtx_fmt_ee (code, mode, src1, src2)); - if (reload_in_progress) + + if (reload_completed + && code == PLUS + && !rtx_equal_p (dst, src1)) { - /* Reload doesn't know about the flags register, and doesn't know that - it doesn't want to clobber it. We can only do this with PLUS. */ - gcc_assert (code == PLUS); - emit_insn (op); - } - else if (reload_completed - && code == PLUS - && !rtx_equal_p (dst, src1)) - { /* This is going to be an LEA; avoid splitting it later. */ emit_insn (op); } @@ -18130,13 +18108,9 @@ ix86_expand_unary_operator (enum rtx_code code, ma /* Emit the instruction. */ op = gen_rtx_SET (VOIDmode, dst, gen_rtx_fmt_e (code, mode, src)); - if (reload_in_progress || code == NOT) - { - /* Reload doesn't know about the flags register, and doesn't know that - it doesn't want to clobber it. */ - gcc_assert (code == NOT); - emit_insn (op); - } + + if (code == NOT) + emit_insn (op); else { clob = gen_rtx_CLOBBER (VOIDmode, gen_rtx_REG (CCmode, FLAGS_REG)); Index: config/i386/predicates.md =================================================================== --- config/i386/predicates.md (revision 222194) +++ config/i386/predicates.md (working copy) @@ -577,7 +577,7 @@ { if (GET_CODE (op) == SUBREG) op = SUBREG_REG (op); - if (reload_in_progress || reload_completed) + if (reload_completed) return REG_OK_FOR_INDEX_STRICT_P (op); else return REG_OK_FOR_INDEX_NONSTRICT_P (op);
Hello! Attached patch removes reload_in_progress checks for x86 (LRA enabled) target. AFAICS, reload_in_progress is never set during the compilation, a watchpoint on this variable didn't trigger for a couple of complex compilations. 2015-04-19 Uros Bizjak <ubizjak@gmail.com> * config/i386/i386.c (set_pic_reg_ever_live): Remove. (legitimize_pic_address): Do not call set_pic_reg_ever_live. (legitimize_tls_address): Ditto. (ix86_expand_move): Ditto. (ix86_expand_binary_operator): Remove reload_in_progress checks. (ix86_expand_unary_operator): Ditto. * config/i386/predicates.md (index_register_operand): Ditto. Patch was bootstrapped on x86_64-linux-gnu and regression tested for x86_64-linux-gnu, i686-linux-gnu with and w/o -fpic. The patch also changes Darwin specific code that can't be tested properly on linux. Instead of leaving a reload_in_progress_check theere, I'd ask someone to bootstrap and regression test the patch on Darwin target. I'll wait for the Darwin regression test results (and possible comments) before committing the patch. Uros.