On 17 May 2013 22:22, Ramana Radhakrishnan <ramra...@arm.com> wrote: > On 05/16/13 07:27, Zhenqiang Chen wrote: >> >> On 15 May 2013 06:31, Ramana Radhakrishnan <ramana....@googlemail.com> >> wrote: >>> >>> Sorry this had dropped off my list of patches to review somehow but >>> anyway here's a first cut review. >>> >>> On Thu, Mar 21, 2013 at 6:58 AM, Zhenqiang Chen >>> <zhenqiang.c...@linaro.org> wrote: >>>> >>>> Hi, >>>> >>>> When shrink-wrap is enabled, the "returns" from simple-return path and >>>> normal return path can be merged. The code is like: >>>> >>>> tst ... >>>> / \ >>>> | push ... >>>> | ... >>>> | pop ... >>>> \ / >>>> bx lr >>>> >>>> If the dwarf info after "pop ..." is incorrect, the dwarf checks will >>>> fail at dwarf2cfi.c: function maybe_record_trace_start. >>>> >>>> /* We ought to have the same state incoming to a given trace no >>>> matter how we arrive at the trace. Anything else means we've >>>> got some kind of optimization error. */ >>>> gcc_checking_assert (cfi_row_equal_p (cur_row, ti->beg_row)); >>>> >>>> The patch is to add epilogue dwarf info to make sure: >>>> >>>> Before "bx lr", >>>> * All registers saved in stack had been "restored". >>>> * .cfi_def_cfa_offset 0 >>>> * .cfi_def_cfa_register is sp even if frame_pointer_needed >>>> >>>> Boot strapped and no make check regression. >>> >>> >>> in what configuration - thumb2 / arm / cortex-a15 / cortex-a9 ? what >>> languages and on what platform ? >> >> >> Bootstrapped both arm and thumb2 on Pandaboard ES and Chromebook. >> Pandaboard ES (cortex-a9): >> --enable-languages=c,c++,fortran,objc,obj-c++ --with-arch=armv7-a >> --with-float=hard --with-fpu=vfpv3-d16 >> --enable-languages=c,c++,fortran,objc,obj-c++ --with-arch=armv7-a >> --with-float=hard --with-fpu=vfpv3-d16 --with-mode=thumb >> >> Chromebook (cortext-a15): >> --enable-languages=c,c++,fortran,objc,obj-c++ --with-arch=armv7-a >> --with-tune=cortex-a15 --with-float=hard --with-fpu=vfpv3-d16 >> --enable-languages=c,c++,fortran,objc,obj-c++ --with-arch=armv7-a >> --with-tune=cortex-a15 --with-float=hard --with-fpu=vfpv3-d16 >> --with-mode=thumb >> >> Regression tests on Pandborad ES for both arm and thumb2 with the same >> configure as bootstrap. >> >>>> >>>> Is it OK? >>> >>> >>> >>>> @@ -25447,9 +25519,15 @@ arm_unwind_emit (FILE * asm_out_file, rtx insn) >>>> handled_one = true; >>>> break; >>> >>> >>>> + /* The INSN is generated in epilogue. It is set as >>>> RTX_FRAME_RELATED_P >>>> + to get correct dwarf info for shrink-wrap. But We should not >>>> emit >>>> + unwind info for it. */ >>> >>> >>> s/dwarf/debug frame. >>> >>> REplace "But We should not emit unwind info for it " with " We should >>> not emit unwind info for it because these are used either for pretend >>> arguments or <put the other case here>. " >>> >>> I think you are correct that there is no need for unwind information >>> as we only have unwind information valid at call points and in this >>> case I wouldn't expect unwind information to need to be exact for >>> pretend arguments >> >> >> Thanks for the comments. The patch is updated and rebased to trunk. > > > > Ok if no regressions. > > regards > Ramana
Thanks, committed to trunk (bootstrap on PandboardES and Chromebook and no make check regression). -Zhenqiang
