HAO CHEN GUI <guih...@linux.ibm.com> writes:
> Hi,
> This patch tries to fix a potential problem which is raised by the patch
> for PR111267. The volatile asm operand tries to be propagated to a single
> set insn with the patch for PR111267. The volatile asm operand might be
> executed for multiple times if the define insn isn't eliminated after
> propagation. Now set_src_cost comparison might reject such propagation.
> But it has the chance to be taken after replacing set_src_cost with insn
> cost. Actually I found the problem in testing my patch which replacing
> set_src_cost with insn_cost in fwprop pass.
>
> Compared to the last version, the check volatile_insn_p is replaced with
> volatile_refs_p in order to check volatile memory reference also.
> https://gcc.gnu.org/pipermail/gcc-patches/2024-February/646482.html
>
> Bootstrapped and tested on x86 and powerpc64-linux BE and LE with no
> regressions. Is it OK for the trunk?
OK, thanks. I'm not sure this fixes a known regression, but IMO the
barrier should be lower for things that fix loss of volatility, since
it's usually so hard to observe the effect in a determinstic way.
Richard
>
> Thanks
> Gui Haochen
>
> ChangeLog
> fwprop: Avoid volatile defines to be propagated
>
> The patch for PR111267 (commit id 86de9b66480b710202a2898cf513db105d8c432f)
> which introduces an exception for propagation on single set insn. The
> propagation which might not be profitable (checked by profitable_p) is still
> allowed to be propagated to single set insn. It has a potential problem
> that a volatile operand might be propagated to a single set insn. If the
> define insn is not eliminated after propagation, the volatile operand will
> be executed for multiple times. This patch fixes the problem by skipping
> volatile set source rtx in propagation.
>
> gcc/
> * fwprop.cc (forward_propagate_into): Return false for volatile set
> source rtx.
>
> gcc/testsuite/
> * gcc.target/powerpc/fwprop-1.c: New.
>
> patch.diff
> diff --git a/gcc/fwprop.cc b/gcc/fwprop.cc
> index 7872609b336..cb6fd6700ca 100644
> --- a/gcc/fwprop.cc
> +++ b/gcc/fwprop.cc
> @@ -854,6 +854,8 @@ forward_propagate_into (use_info *use, bool reg_prop_only
> = false)
>
> rtx dest = SET_DEST (def_set);
> rtx src = SET_SRC (def_set);
> + if (volatile_refs_p (src))
> + return false;
>
> /* Allow propagations into a loop only for reg-to-reg copies, since
> replacing one register by another shouldn't increase the cost.
> diff --git a/gcc/testsuite/gcc.target/powerpc/fwprop-1.c
> b/gcc/testsuite/gcc.target/powerpc/fwprop-1.c
> new file mode 100644
> index 00000000000..07b207f980c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/fwprop-1.c
> @@ -0,0 +1,15 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -fdump-rtl-fwprop1-details" } */
> +/* { dg-final { scan-rtl-dump-not "propagating insn" "fwprop1" } } */
> +
> +/* Verify that volatile asm operands doesn't be propagated. */
> +long long foo ()
> +{
> + long long res;
> + __asm__ __volatile__(
> + ""
> + : "=r" (res)
> + :
> + : "memory");
> + return res;
> +}
