| Message ID | 54B28270.8000208@sunrus.com.cn |
|---|---|
| State | New |
| Headers | show |
On 01/11/15 07:02, Chen Gang S wrote: > The related commit "1a1ed14 config/h8300: Use rtx_insn" gives an extra > check for rtx, which will cause building libgcc break, after regress it, > it can still generate the correct assemble code. > > The related information is below: > > [root@localhost libgcc]# cat libgcc2.i > typedef int DItype __attribute__ ((mode (DI))); > DItype __muldi3 (DItype u, DItype v) > { > return u + v; > } > [root@localhost libgcc]# /upstream/build-gcc-h8300/gcc/cc1 -ms -O2 libgcc2.i > __muldi3 > Analyzing compilation unit > Performing interprocedural optimizations > <*free_lang_data> <visibility> <build_ssa_passes> <chkp_passes> <opt_local_passes> <free-inline-summary> <emutls> <whole-program> <profile_estimate> <icf> <devirt> <cp> <inline> <pure-const> <static-var> <single-use> <comdats>Assembling functions: > __muldi3 > libgcc2.i: In function '__muldi3': > libgcc2.i:5:1: internal compiler error: in as_a, at is-a.h:192 > } This indicates a violation of the type safety invariants we're adding to GCC. Simply changing the code to use rtx rather than rtx_insn is probably a step in the wrong direction. Part of the problem here is that RTX_FRAME_RELATED_P is valid on both rtx_insn and rtx objects. That's something we'll have to fix as the type safety work moves forward, assuming we continue towards the goal of totally separating rtx and rtx_insn objects. Returning to the code in h8300.c, we have "F" which assumes its argument is an rtx_insn. We should never be calling "F" will anything other than an rtx_insn argument. The calls from "Fpa" are the only violators of that invariant. Given that the vectors inside the PARALLEL will always be rtx objects and that we always want to set RTX_FRAME_RELATED on those objects, it seems that we could just replace the call to "F" in "Fpa" with RTX_FRAME_RELATED_P (XVECEXP (par, 0, i)) = 1; That simplifies the code and removes a bogus as_a cast. Can you try that and report back to me? Thanks, Jeff ps. Someone should have chastised DJ for using such poor function names as "F" and "Fpa". If you wanted to clean that up and use more descriptive function names, that would be appreciated as a separate patch.
On 01/12/15 10:01, Jeff Law wrote: > This indicates a violation of the type safety invariants we're adding to > GCC. Simply changing the code to use rtx rather than rtx_insn is > probably a step in the wrong direction. > > Part of the problem here is that RTX_FRAME_RELATED_P is valid on both > rtx_insn and rtx objects. That's something we'll have to fix as the > type safety work moves forward, assuming we continue towards the goal of > totally separating rtx and rtx_insn objects. > > Returning to the code in h8300.c, we have "F" which assumes its argument > is an rtx_insn. We should never be calling "F" will anything other than > an rtx_insn argument. The calls from "Fpa" are the only violators of > that invariant. > > Given that the vectors inside the PARALLEL will always be rtx objects > and that we always want to set RTX_FRAME_RELATED on those objects, it > seems that we could just replace the call to "F" in "Fpa" with > > RTX_FRAME_RELATED_P (XVECEXP (par, 0, i)) = 1; > > > That simplifies the code and removes a bogus as_a cast. > > Can you try that and report back to me? Nevermind. I went ahead and made this change and verified that libgcc, libquadmath and newlib would build for the h8300-elf configuration. Jeff
On 1/13/15 01:32, Jeff Law wrote: > On 01/12/15 10:01, Jeff Law wrote: >> This indicates a violation of the type safety invariants we're adding to >> GCC. Simply changing the code to use rtx rather than rtx_insn is >> probably a step in the wrong direction. >> >> Part of the problem here is that RTX_FRAME_RELATED_P is valid on both >> rtx_insn and rtx objects. That's something we'll have to fix as the >> type safety work moves forward, assuming we continue towards the goal of >> totally separating rtx and rtx_insn objects. >> >> Returning to the code in h8300.c, we have "F" which assumes its argument >> is an rtx_insn. We should never be calling "F" will anything other than >> an rtx_insn argument. The calls from "Fpa" are the only violators of >> that invariant. >> Oh, what you said above sound reasonable to me. "F" is also used by others which pass rtx_insn to "F". >> Given that the vectors inside the PARALLEL will always be rtx objects >> and that we always want to set RTX_FRAME_RELATED on those objects, it >> seems that we could just replace the call to "F" in "Fpa" with >> >> RTX_FRAME_RELATED_P (XVECEXP (par, 0, i)) = 1; >> >> >> That simplifies the code and removes a bogus as_a cast. >> Yeah, for me, it is a good idea. >> Can you try that and report back to me? > Nevermind. I went ahead and made this change and verified that libgcc, libquadmath and newlib would build for the h8300-elf configuration. > Thank you for your work.
diff --git a/gcc/config/h8300/h8300.c b/gcc/config/h8300/h8300.c index fe85df5..994c38f 100644 --- a/gcc/config/h8300/h8300.c +++ b/gcc/config/h8300/h8300.c @@ -506,8 +506,8 @@ byte_reg (rtx x, int b) && !crtl->is_leaf))) /* We use this to wrap all emitted insns in the prologue. */ -static rtx_insn * -F (rtx_insn *x, bool set_it) +static rtx +F (rtx x, bool set_it) { if (set_it) RTX_FRAME_RELATED_P (x) = 1; @@ -527,7 +527,7 @@ Fpa (rtx par) int i; for (i = 0; i < len; i++) - F (as_a <rtx_insn *> (XVECEXP (par, 0, i)), true); + F (XVECEXP (par, 0, i), true); return par; }
The related commit "1a1ed14 config/h8300: Use rtx_insn" gives an extra check for rtx, which will cause building libgcc break, after regress it, it can still generate the correct assemble code. The related information is below: [root@localhost libgcc]# cat libgcc2.i typedef int DItype __attribute__ ((mode (DI))); DItype __muldi3 (DItype u, DItype v) { return u + v; } [root@localhost libgcc]# /upstream/build-gcc-h8300/gcc/cc1 -ms -O2 libgcc2.i __muldi3 Analyzing compilation unit Performing interprocedural optimizations <*free_lang_data> <visibility> <build_ssa_passes> <chkp_passes> <opt_local_passes> <free-inline-summary> <emutls> <whole-program> <profile_estimate> <icf> <devirt> <cp> <inline> <pure-const> <static-var> <single-use> <comdats>Assembling functions: __muldi3 libgcc2.i: In function '__muldi3': libgcc2.i:5:1: internal compiler error: in as_a, at is-a.h:192 } ^ 0xce2aef as_a<rtx_insn*, rtx_def> ../../gcc/gcc/is-a.h:192 0xce2aef Fpa ../../gcc/gcc/config/h8300/h8300.c:530 0xce2aef h8300_push_pop ../../gcc/gcc/config/h8300/h8300.c:724 0xce33e3 h8300_push_pop ../../gcc/gcc/config/h8300/h8300.c:869 0xce33e3 h8300_expand_prologue() ../../gcc/gcc/config/h8300/h8300.c:890 0xcee69a gen_prologue() ../../gcc/gcc/config/h8300/h8300.md:2651 0x7c0b19 thread_prologue_and_epilogue_insns() ../../gcc/gcc/function.c:5923 0x7c1032 rest_of_handle_thread_prologue_and_epilogue ../../gcc/gcc/function.c:6493 0x7c1032 execute ../../gcc/gcc/function.c:6531 After this patch, it can generate the correct assembly code: [root@localhost libgcc]# h8300-gchen-elf-gcc -ms -O2 -S libgcc2.i [root@localhost libgcc]# cat ./libgcc2.s .file "libgcc2.i" .h8300s .section .text .align 1 .global ___muldi3 ___muldi3: mov.l er6,@-er7 mov.l er7,er6 stm.l er4-er5,@-er7 sub.l #12,er7 mov.l er0,er2 mov.l er1,er3 mov.l @(8,er6),er0 mov.l er0,@(-20,er6) mov.l @(12,er6),er0 mov.l er0,@(-16,er6) mov.l er0,er5 add.l er1,er5 sub.l er1,er1 add.b #1,r1l cmp.l er3,er5 blo .L2 sub.l er1,er1 .L2: mov.l @(-20,er6),er4 add.l er2,er4 mov.l er1,er0 add.l er4,er0 mov.l er5,er1 add.l #12,er7 ldm.l @er7+,er4-er5 mov.l @er7+,er6 rts .size ___muldi3, .-___muldi3 .ident "GCC: (GNU) 5.0.0 20150109 (experimental)" .end For mode(DI), it generates 64-bit integer, so it uses er0 and er1 as parameter 1, and stack (8,er6) and (12,er6) for parameter 2, return value is er0 and er1. And the internal algorithim is also correct. 2015-01-11 Chen Gang <gang.chen.5i5j@gmail.com> * config/h8300/h8300.c (F): Use rtx instead of rtx_insn *. (Fpa): Remove additional check by rtx_insn *. --- gcc/config/h8300/h8300.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)