This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 15/18] make nonlocal_goto_handler_labels a vec
- From: Trevor Saunders <tbsaunde at tbsaunde dot org>
- To: Bernd Schmidt <bschmidt at redhat dot com>
- Cc: tbsaunde+gcc at tbsaunde dot org, gcc-patches at gcc dot gnu dot org
- Date: Mon, 25 Apr 2016 09:43:45 -0400
- Subject: Re: [PATCH 15/18] make nonlocal_goto_handler_labels a vec
- Authentication-results: sourceware.org; auth=none
- References: <1461133342-10794-1-git-send-email-tbsaunde+gcc at tbsaunde dot org> <1461133342-10794-16-git-send-email-tbsaunde+gcc at tbsaunde dot org> <571E10EF dot 5060100 at redhat dot com>
On Mon, Apr 25, 2016 at 02:43:27PM +0200, Bernd Schmidt wrote:
> On 04/20/2016 08:22 AM, tbsaunde+gcc@tbsaunde.org wrote:
> >- remove_node_from_insn_list (insn, &nonlocal_goto_handler_labels);
> >+
> >+ unsigned int len = vec_safe_length (nonlocal_goto_handler_labels);
> >+ for (unsigned int i = 0; i < len; i++)
> >+ if ((*nonlocal_goto_handler_labels)[i] == insn)
> >+ {
> >+ nonlocal_goto_handler_labels->ordered_remove (i);
> >+ break;
> >+ }
>
> Why not unordered_remove?
I believe it was to keep the same behavior as what was done with lists.
I'm not sure that matters, but I don't think that I understand
everything involved well enough to say it definitely does not matter so
I wanted to be safe and avoid introducing bugs.
> Should there be a vec-based version of remove-node_from_*_list?
Maybe, but I will echo Richard's concern about making O(N) operations
easy.
> >+ rtx_insn *temp;
> >+ unsigned int i;
> >+ FOR_EACH_VEC_SAFE_ELT_REVERSE (nonlocal_goto_handler_labels, i, temp)
> >+ BLOCK_FOR_INSN (temp)->flags |= BB_NON_LOCAL_GOTO_TARGET;
>
> Indentation looks wrong.
Sorry
> >
> > /* Process non-local goto edges. */
> > if (can_nonlocal_goto (insn))
> >- for (rtx_insn_list *lab = nonlocal_goto_handler_labels;
> >- lab;
> >- lab = lab->next ())
> >- maybe_record_trace_start_abnormal (lab->insn (), insn);
> >+ {
>
> Unnecessary brace?
Will check
> > /* Re-insert the EH_REGION notes. */
> >- if (eh_note || (was_call && nonlocal_goto_handler_labels))
> >+ if (eh_note || (was_call && vec_safe_length (nonlocal_goto_handler_labels)))
>
> I'm not a big fan of omitting the > 0 and using the integer as a boolean.
> Multiple occurrences.
ok, I spend a lot of time working on code where that is the style, so
its habbit sorry.
Trev