[patch] handle casesi dispatch insns in create_trace_edges
Jeff Law
law@redhat.com
Fri Aug 9 21:23:00 GMT 2019
On 8/9/19 11:07 AM, Olivier Hainque wrote:
> Hello,
>
> The attached patch is a proposal to plug a hole in create_trace_edges
> (dwarf2cfi.c), which doesn't handle casesi dispatch insns.
>
> The visible misbehavior we observed is a failure in a cross configuration
> of a recent acats test for Ada, a very simplified sketch of which is provided
> below.
>
> This was with gcc-7 on a port which has been deprecated since then, but ISTM
> the problem remains latent for other ports.
>
> Essentially, we had a jump insn like:
>
> if X <= 4 -- for case values
> set pc *(&label_59 + kind * 4) -- 0 .. 4
> else
> set pc &label_151
>
> for the case statement, and the tablejump_p processing code in
> create_trace_edges only gets through the first 5 possible targets.
>
> The missing edge in the re-created control-flow graph eventually materialized
> as missing .cfi_remember_state/.cfi_restore_state pairs in the output, which
> resulted in bogus exception handling behavior.
>
> The insn pattern corresponds to the one handled in patch_jump_insn
> (cfgrtl.c). The proposed patch extracts the pattern recognition code
> in a separate function and uses it in both patch_jump_insn and
> create_trace_edges.
>
> This fixed our problem on the cross port (arm-vxworks6) and we
> have been running with it for all our gcc-7 and gcc-8 ports since
> then, about 6 months ago now.
>
> It also bootstraps and regression tests fine with mainline
> on x86_64-linux.
>
> Ok, to commit ?
>
> Thanks in advance!
>
> With Kind Regards,
>
> Olivier
>
> 2019-08-09 Olivier Hainque <hainque@adacore.com>
>
> * rtl.h (tablejump_casesi_pattern): New helper, casesi
> recognition logic originating from code in cfgrtl.c.
> * cfgrtl.c (patch_jump_insn): Use it.
> * dwarf2cfi.c (create_trace_edges): Handle casesi patterns.
Is there a reason to think the routine is performance critical enough to
be inlined? If not it would make more sense to me to put it into rtl.c
with just a declaration in rtl.h
So if it is performance critical, then the patch is OK as-is. If not,
moving the implementation into rtl.c with a declaration in rtl.h should
be considered pre-approved -- just post it here for archival purposes.
Thanks,
jeff
More information about the Gcc-patches
mailing list