Christoph Müllner <christoph.muell...@vrull.eu> writes:
> insn_info::has_been_deleted () is documented to return true if an
> instruction is deleted. Such instructions have their `volatile` bit set,
> which can be tested via rtx_insn::deleted ().
>
> The current condition for insn_info::has_been_deleted () is:
> * m_rtl is not NULL: this can't happen as no member of insn_info
> changes this pointer.
Yeah, it's invariant after creation, but it starts off null for some
artificial instructions:
// Return the underlying RTL insn. This instruction is null if is_phi ()
// or is_bb_end () are true. The instruction is a basic block note if
// is_bb_head () is true.
rtx_insn *rtl () const { return m_rtl; }
So I think we should keep the null check. (But then is_call and is_jump
should check m_rtl is nonnull too -- that's preapproved if you want to
do it while you're here.)
> * !INSN_P (m_rtl): this will likely fail for rtx_insn objects and
> does not test the `volatile` bit.
Because of the need to stage multiple simultaneous changes, rtl-ssa first
uses set_insn_deleted to convert an insn to a NOTE_INSN_DELETED note,
then uses remove_insn to remove the underlying instruction. It doesn't
use delete_insn directly. The call to remove_insn is fairly recent;
the original code just used set_insn_deleted, but not removing the notes
caused trouble for later passes.
The test was therefore supposed to be checking whether set_insn_deleted
had been called. It should also have checked the note kind though.
However, I agree that testing the deleted flag would be better.
For that to work, we'd need to set the deleted flag here:
if (rtx_insn *rtl = insn->rtl ())
::remove_insn (rtl); // Remove the underlying RTL insn.
as well as calling remove_insn. Alternatively (and better), we could
try converting ::remove_insn to ::delete_insn.
Thanks,
Richard
> This patch drops these conditions and calls m_rtl->deleted () instead.
>
> The impact of this change is minimal as insn_info::has_been_deleted
> is only called in insn_info::print_full.
>
> Bootstrapped and regtested x86_64-linux.
>
> gcc/ChangeLog:
>
> * rtl-ssa/insns.h: Fix implementation of has_been_deleted ().
>
> Signed-off-by: Christoph Müllner <christoph.muell...@vrull.eu>
> ---
> gcc/rtl-ssa/insns.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/rtl-ssa/insns.h b/gcc/rtl-ssa/insns.h
> index d89dfc5c3f66..bb3f52efa83a 100644
> --- a/gcc/rtl-ssa/insns.h
> +++ b/gcc/rtl-ssa/insns.h
> @@ -186,7 +186,7 @@ public:
> // Return true if the instruction was a real instruction but has now
> // been deleted. In this case the instruction is no longer part of
> // the SSA information.
> - bool has_been_deleted () const { return m_rtl && !INSN_P (m_rtl); }
> + bool has_been_deleted () const { return m_rtl->deleted (); }
>
> // Return true if the instruction is a debug instruction (and thus
> // also a real instruction).
