diff mbox series

loop-invariant: Treat inline-asm conditional trapping [PR102150]

Message ID 20250212083646.297869-1-quic_apinski@quicinc.com
State Committed
Headers
Series loop-invariant: Treat inline-asm conditional trapping [PR102150] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-aarch64-bootstrap success Build passed
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-arm-bootstrap success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 fail Patch failed to apply

Commit Message

Andrew Pinski Feb. 12, 2025, 8:36 a.m. UTC 
  So inline-asm is known not to trap BUT it can have undefined behavior
if made executed speculatively. This fixes the loop invariant pass to
treat it similarly as trapping cases. If the inline-asm could be executed
always, then it will be pulled out of the loop; otherwise it will be kept
inside the loop.

Bootstrapped and tested on x86_64-linux-gnu.

gcc/ChangeLog:

	* loop-invariant.cc (find_invariant_insn): Treat inline-asm similar to
	trapping instruction and only move them if always executed.

Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
---
 gcc/loop-invariant.cc | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Richard Biener Feb. 12, 2025, 8:57 a.m. UTC | #1 
On Wed, Feb 12, 2025 at 9:41 AM Andrew Pinski <quic_apinski@quicinc.com> wrote:
>
> So inline-asm is known not to trap BUT it can have undefined behavior
> if made executed speculatively. This fixes the loop invariant pass to
> treat it similarly as trapping cases. If the inline-asm could be executed
> always, then it will be pulled out of the loop; otherwise it will be kept
> inside the loop.
>
> Bootstrapped and tested on x86_64-linux-gnu.
>
> gcc/ChangeLog:
>
>         * loop-invariant.cc (find_invariant_insn): Treat inline-asm similar to
>         trapping instruction and only move them if always executed.
>
> Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> ---
>  gcc/loop-invariant.cc | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/gcc/loop-invariant.cc b/gcc/loop-invariant.cc
> index bcb52bb9c76..79a4c39dfb0 100644
> --- a/gcc/loop-invariant.cc
> +++ b/gcc/loop-invariant.cc
> @@ -1123,6 +1123,11 @@ find_invariant_insn (rtx_insn *insn, bool always_reached, bool always_executed)
>    if (may_trap_or_fault_p (PATTERN (insn)) && !always_reached)
>      return;
>
> +  /* inline-asm that is not always executed cannot be moved
> +     as it might trap. */

as it might conditionally trap?

OK.

Thanks,
Richard.

> +  if (!always_reached && asm_noperands (PATTERN (insn)) >= 0)
> +    return;
> +
>    depends_on = BITMAP_ALLOC (NULL);
>    if (!check_dependencies (insn, depends_on))
>      {
> --
> 2.43.0
>
Andrew Pinski Feb. 13, 2025, 3:29 a.m. UTC | #2 
On Wed, Feb 12, 2025 at 1:00 AM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Wed, Feb 12, 2025 at 9:41 AM Andrew Pinski <quic_apinski@quicinc.com> wrote:
> >
> > So inline-asm is known not to trap BUT it can have undefined behavior
> > if made executed speculatively. This fixes the loop invariant pass to
> > treat it similarly as trapping cases. If the inline-asm could be executed
> > always, then it will be pulled out of the loop; otherwise it will be kept
> > inside the loop.
> >
> > Bootstrapped and tested on x86_64-linux-gnu.
> >
> > gcc/ChangeLog:
> >
> >         * loop-invariant.cc (find_invariant_insn): Treat inline-asm similar to
> >         trapping instruction and only move them if always executed.
> >
> > Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> > ---
> >  gcc/loop-invariant.cc | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/gcc/loop-invariant.cc b/gcc/loop-invariant.cc
> > index bcb52bb9c76..79a4c39dfb0 100644
> > --- a/gcc/loop-invariant.cc
> > +++ b/gcc/loop-invariant.cc
> > @@ -1123,6 +1123,11 @@ find_invariant_insn (rtx_insn *insn, bool always_reached, bool always_executed)
> >    if (may_trap_or_fault_p (PATTERN (insn)) && !always_reached)
> >      return;
> >
> > +  /* inline-asm that is not always executed cannot be moved
> > +     as it might trap. */
>
> as it might conditionally trap?

Yes and I changed the comment to be:
  /* inline-asm that is not always executed cannot be moved
     as it might conditionally trap. */

And pushed with that change.

Thanks,
Andrew Pinski

>
> OK.
>
> Thanks,
> Richard.
>
> > +  if (!always_reached && asm_noperands (PATTERN (insn)) >= 0)
> > +    return;
> > +
> >    depends_on = BITMAP_ALLOC (NULL);
> >    if (!check_dependencies (insn, depends_on))
> >      {
> > --
> > 2.43.0
> >
diff mbox series

Patch

diff --git a/gcc/loop-invariant.cc b/gcc/loop-invariant.cc
index bcb52bb9c76..79a4c39dfb0 100644
--- a/gcc/loop-invariant.cc
+++ b/gcc/loop-invariant.cc
@@ -1123,6 +1123,11 @@  find_invariant_insn (rtx_insn *insn, bool always_reached, bool always_executed)
   if (may_trap_or_fault_p (PATTERN (insn)) && !always_reached)
     return;
 
+  /* inline-asm that is not always executed cannot be moved
+     as it might trap. */
+  if (!always_reached && asm_noperands (PATTERN (insn)) >= 0)
+    return;
+
   depends_on = BITMAP_ALLOC (NULL);
   if (!check_dependencies (insn, depends_on))
     {