Re: [PATCH] x86: Update BB_HEAD when aligning BB_HEAD

[H.J. Lu]

On Mon, Aug 12, 2024 at 12:25 AM Jakub Jelinek <ja...@redhat.com> wrote:
>
> On Thu, Aug 08, 2024 at 06:53:10PM -0700, H.J. Lu wrote:
> > When we emit .p2align to align BB_HEAD, we must update BB_HEAD.  Otherwise
> > ENDBR will be inserted as the wrong place.
> >
> > gcc/
> >
> >       PR target/116174
> >       * config/i386/i386.cc (ix86_align_loops): Update BB_HEAD when
> >       aligning BB_HEAD
> >
> > gcc/testsuite/
> >
> >       PR target/116174
> >       * gcc.target/i386/pr116174.c: New test.
> >
> > Signed-off-by: H.J. Lu <hjl.to...@gmail.com>
> > ---
> >  gcc/config/i386/i386.cc                  |  7 +++++--
> >  gcc/testsuite/gcc.target/i386/pr116174.c | 12 ++++++++++++
> >  2 files changed, 17 insertions(+), 2 deletions(-)
> >  create mode 100644 gcc/testsuite/gcc.target/i386/pr116174.c
> >
> > diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
> > index 77c441893b4..ec6cc5e3548 100644
> > --- a/gcc/config/i386/i386.cc
> > +++ b/gcc/config/i386/i386.cc
> > @@ -23528,8 +23528,11 @@ ix86_align_loops ()
> >
> >         if (padding_p && detect_tight_loop_p)
> >           {
> > -           emit_insn_before (gen_max_skip_align (GEN_INT (ceil_log2 
> > (size)),
> > -                                                 GEN_INT (0)), label);
> > +           rtx_insn *align =
>
> = can't go at the end of line.
>
> > +             emit_insn_before (gen_max_skip_align (GEN_INT (ceil_log2 
> > (size)),
> > +                                                   GEN_INT (0)), label);
> > +           if (BB_HEAD (bb) == label)
> > +             BB_HEAD (bb) = align;
>
> This is wrong.  As documented, BB_HEAD needs to be either a CODE_LABEL, or
> NOTE_INSN_BASIC_BLOCK.

ira.cc has

          new_insn = emit_insn_before (PATTERN (def_insn), use_insn);
          REG_NOTES (new_insn) = REG_NOTES (def_insn);
          REG_NOTES (def_insn) = 0;
          /* Rescan it to process the notes.  */
          df_insn_rescan (new_insn);

          /* Make sure this insn is recognized before reload begins,
             otherwise eliminate_regs_in_insn will die.  */
          INSN_CODE (new_insn) = INSN_CODE (def_insn);

          delete_insn (def_insn);

          XEXP (reg_equiv[regno].init_insns, 0) = new_insn;

          REG_BASIC_BLOCK (regno) = use_bb->index;
          REG_N_CALLS_CROSSED (regno) = 0;

          if (use_insn == BB_HEAD (use_bb))
            BB_HEAD (use_bb) = new_insn;

new_insn isn't CODE_LABEL nor NOTE_INSN_BASIC_BLOCK.
Should it be changed?

> E.g. emit-rtl.cc says
>       /* Should not happen as first in the BB is always either NOTE or
>          LABEL.  */
>
>         Jakub
>


-- 
H.J.