On Mon, Oct 11, 2021 at 1:20 AM apinski--- via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> From: Andrew Pinski <apin...@marvell.com>
>
> So here is the GCC 10 branch version which fixes the wrong code.
> The problem is we create a negation of an one bit signed integer type
> which is undefined if the value was -1.
> This is not needed for GCC 11 branch since the case is handled differently
> there and has been fixed there (and the trunk has now been fixed too).
> So for one bit types, there is no reason to create the negation so just
> setting neg to false for them, just works.
>
> OK? Bootstrapped and tested on x86_64-linux-gnu.
OK.
Thanks,
Richard.
> PR tree-optimization/102622
>
> gcc/ChangeLog:
>
> * tree-ssa-phiopt.c (conditional_replacement): Set neg
> to false for one bit signed types.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.c-torture/execute/bitfld-10.c: New test.
> ---
> gcc/testsuite/gcc.c-torture/execute/bitfld-10.c | 24 ++++++++++++++++++++++++
> gcc/tree-ssa-phiopt.c | 5 ++++-
> 2 files changed, 28 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.c-torture/execute/bitfld-10.c
>
> diff --git a/gcc/testsuite/gcc.c-torture/execute/bitfld-10.c
> b/gcc/testsuite/gcc.c-torture/execute/bitfld-10.c
> new file mode 100644
> index 0000000..bdbf573
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/execute/bitfld-10.c
> @@ -0,0 +1,24 @@
> +/* PR tree-optimization/102622 */
> +/* Wrong code introduced due to phi-opt
> + introducing undefined signed interger overflow
> + with one bit signed integer negation. */
> +
> +struct f{signed t:1;};
> +int g(struct f *a, int t) __attribute__((noipa));
> +int g(struct f *a, int t)
> +{
> + if (t)
> + a->t = -1;
> + else
> + a->t = 0;
> + int t1 = a->t;
> + if (t1) return 1;
> + return t1;
> +}
> +
> +int main(void)
> +{
> + struct f a;
> + if (!g(&a, 1)) __builtin_abort();
> + return 0;
> +}
> diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
> index 9ed26a3..a6c197d 100644
> --- a/gcc/tree-ssa-phiopt.c
> +++ b/gcc/tree-ssa-phiopt.c
> @@ -770,9 +770,12 @@ conditional_replacement (basic_block cond_bb,
> basic_block middle_bb,
> if ((integer_zerop (arg0) && integer_onep (arg1))
> || (integer_zerop (arg1) && integer_onep (arg0)))
> neg = false;
> + /* For signed one bit types, the negation is not needed and
> + should be avoided and is the same as 1 case for non-signed
> + one bit types. */
> else if ((integer_zerop (arg0) && integer_all_onesp (arg1))
> || (integer_zerop (arg1) && integer_all_onesp (arg0)))
> - neg = true;
> + neg = TYPE_PRECISION (TREE_TYPE (arg0)) != 1;
> else
> return false;
>
> --
> 1.8.3.1
>
