[Bug c/94366] New: OpenMP Parallel Reduction with "&&" operator does not compute correct result

[Bug c/94366] New: OpenMP Parallel Reduction with "&&" operator does not compute correct result

[satzeni at nvidia dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94366

            Bug ID: 94366
           Summary: OpenMP Parallel Reduction with "&&" operator does not
                    compute correct result
           Product: gcc
           Version: 9.3.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: satzeni at nvidia dot com
  Target Milestone: ---

Created attachment 48137
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48137&action=edit
reproducer

Consider this program:

#include <assert.h>
#include <omp.h>
#include <stdio.h>

static int expect = 1;
static int result;

int main() {
    int a = 2;  /* or any another even number */

#pragma omp parallel reduction(&& : a)
    {
        a = a && 1;
    }
    result = a ? 1 : 0;
    assert(result == 1);
}

Compiler and run with:

$ gcc -fopenmp test.c
$ OMP_NUM_THREADS=2 ./a.out

GCC is not correctly computing the logical-and reduction on variable "a".

Inside the parallel region "a" value is 1, but right after the parallel region
is 0 failing the assertion. The correct result should be 1 as the initial value
is 2 and the logical-and between 2 && 1 should be non zero.

Looking at the assembly code, GCC is using a bitwise “and” instruction to
perform the “&&” operation:

<main._omp_fn.0+26>    mov    (%rbx),%edx
<main._omp_fn.0+28>    jmp    0x400782 <main._omp_fn.0+34>
<main._omp_fn.0+30>    xchg   %ax,%ax
<main._omp_fn.0+32>    mov    %eax,%edx
<main._omp_fn.0+34>    mov    %edx,%ecx
<main._omp_fn.0+36>    mov    %edx,%eax
<main._omp_fn.0+38>    and    $0x1,%ecx
<main._omp_fn.0+41>    lock cmpxchg %ecx,(%rbx)

However the the value of “a” is not reduced to “1” before it is passed to the
parallel region.

<main+15>              movl   $0x2,(%rsp)
<main+22>              callq  0x4005c0 <GOMP_parallel_start@plt>

[Bug c/94366] OpenMP Parallel Reduction with "&&" operator does not compute correct result

[pal0009 at uah dot edu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94366

Phillip Lane <pal0009 at uah dot edu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pal0009 at uah dot edu

--- Comment #1 from Phillip Lane <pal0009 at uah dot edu> ---
I have reproduced this bug using gcc/9.3.0 on Ubuntu 20.04. A preliminary look
shows that it appears to be in the backend and isn't an issue with the parser.
I'll look at this more closely in the following week to see if I can't resolve
this.

[Bug middle-end/94366] OpenMP Parallel Reduction with "&&" operator does not compute correct result

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94366

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org
   Last reconfirmed|                            |2021-06-30

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 51088
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51088&action=edit
gcc12-pr94366.patch

Untested fix.

[Bug middle-end/94366] OpenMP Parallel Reduction with "&&" operator does not compute correct result

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94366

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:91c771ec8a3b649765de3e0a7b04cf946c6649ef

commit r12-1947-g91c771ec8a3b649765de3e0a7b04cf946c6649ef
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Jul 1 08:55:49 2021 +0200

    openmp - Fix up && and || reductions [PR94366]

    As the testcase shows, the special treatment of && and || reduction
combiners
    where we expand them as omp_out = (omp_out != 0) && (omp_in != 0) (or with
||)
    is not needed just for &&/|| on floating point or complex types, but for
all
    &&/|| reductions - when expanded as omp_out = omp_out && omp_in (not in C
but
    GENERIC) it is actually gimplified into NOP_EXPRs to bool from both
operands,
    which turns non-zero values multiple of 2 into 0 rather than 1.

    This patch just treats all &&/|| the same and furthermore uses bool type
    instead of int for the comparisons.

    2021-07-01  Jakub Jelinek  <jakub@redhat.com>

            PR middle-end/94366
    gcc/
            * omp-low.c (lower_rec_input_clauses): Rename is_fp_and_or to
            is_truth_op, set it for TRUTH_*IF_EXPR regardless of new_var's
type,
            use boolean_type_node instead of integer_type_node as NE_EXPR type.
            (lower_reduction_clauses): Likewise.
    libgomp/
            * testsuite/libgomp.c-c++-common/pr94366.c: New test.

[Bug middle-end/94366] OpenMP Parallel Reduction with "&&" operator does not compute correct result

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94366

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed on the trunk so far.

[Bug middle-end/94366] OpenMP Parallel Reduction with "&&" operator does not compute correct result

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94366

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:9119f51f40226526bfbb79dec07ef39ec8d87466

commit r11-8774-g9119f51f40226526bfbb79dec07ef39ec8d87466
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Jul 1 08:55:49 2021 +0200

    openmp - Fix up && and || reductions [PR94366]

    As the testcase shows, the special treatment of && and || reduction
combiners
    where we expand them as omp_out = (omp_out != 0) && (omp_in != 0) (or with
||)
    is not needed just for &&/|| on floating point or complex types, but for
all
    &&/|| reductions - when expanded as omp_out = omp_out && omp_in (not in C
but
    GENERIC) it is actually gimplified into NOP_EXPRs to bool from both
operands,
    which turns non-zero values multiple of 2 into 0 rather than 1.

    This patch just treats all &&/|| the same and furthermore uses bool type
    instead of int for the comparisons.

    2021-07-01  Jakub Jelinek  <jakub@redhat.com>

            PR middle-end/94366
    gcc/
            * omp-low.c (lower_rec_input_clauses): Rename is_fp_and_or to
            is_truth_op, set it for TRUTH_*IF_EXPR regardless of new_var's
type,
            use boolean_type_node instead of integer_type_node as NE_EXPR type.
            (lower_reduction_clauses): Likewise.
    libgomp/
            * testsuite/libgomp.c-c++-common/pr94366.c: New test.

    (cherry picked from commit 91c771ec8a3b649765de3e0a7b04cf946c6649ef)

[Bug middle-end/94366] OpenMP Parallel Reduction with "&&" operator does not compute correct result

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94366

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:9081951acee45682821746d5fde7816c5d7f526a

commit r10-10634-g9081951acee45682821746d5fde7816c5d7f526a
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Jul 1 08:55:49 2021 +0200

    openmp - Fix up && and || reductions [PR94366]

    As the testcase shows, the special treatment of && and || reduction
combiners
    where we expand them as omp_out = (omp_out != 0) && (omp_in != 0) (or with
||)
    is not needed just for &&/|| on floating point or complex types, but for
all
    &&/|| reductions - when expanded as omp_out = omp_out && omp_in (not in C
but
    GENERIC) it is actually gimplified into NOP_EXPRs to bool from both
operands,
    which turns non-zero values multiple of 2 into 0 rather than 1.

    This patch just treats all &&/|| the same and furthermore uses bool type
    instead of int for the comparisons.

    2021-07-01  Jakub Jelinek  <jakub@redhat.com>

            PR middle-end/94366
    gcc/
            * omp-low.c (lower_rec_input_clauses): Rename is_fp_and_or to
            is_truth_op, set it for TRUTH_*IF_EXPR regardless of new_var's
type,
            use boolean_type_node instead of integer_type_node as NE_EXPR type.
            (lower_reduction_clauses): Likewise.
    libgomp/
            * testsuite/libgomp.c-c++-common/pr94366.c: New test.

    (cherry picked from commit 91c771ec8a3b649765de3e0a7b04cf946c6649ef)

[Bug middle-end/94366] OpenMP Parallel Reduction with "&&" operator does not compute correct result

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94366

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:26c58b164fd78db4ce25f0183ed0225c08018f3f

commit r9-10091-g26c58b164fd78db4ce25f0183ed0225c08018f3f
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Jul 1 08:55:49 2021 +0200

    openmp - Fix up && and || reductions [PR94366]

    As the testcase shows, the special treatment of && and || reduction
combiners
    where we expand them as omp_out = (omp_out != 0) && (omp_in != 0) (or with
||)
    is not needed just for &&/|| on floating point or complex types, but for
all
    &&/|| reductions - when expanded as omp_out = omp_out && omp_in (not in C
but
    GENERIC) it is actually gimplified into NOP_EXPRs to bool from both
operands,
    which turns non-zero values multiple of 2 into 0 rather than 1.

    This patch just treats all &&/|| the same and furthermore uses bool type
    instead of int for the comparisons.

    2021-07-01  Jakub Jelinek  <jakub@redhat.com>

            PR middle-end/94366
    gcc/
            * omp-low.c (lower_rec_input_clauses): Rename is_fp_and_or to
            is_truth_op, set it for TRUTH_*IF_EXPR regardless of new_var's
type,
            use boolean_type_node instead of integer_type_node as NE_EXPR type.
            (lower_reduction_clauses): Likewise.
    libgomp/
            * testsuite/libgomp.c-c++-common/pr94366.c: New test.

    (cherry picked from commit 91c771ec8a3b649765de3e0a7b04cf946c6649ef)

[Bug middle-end/94366] OpenMP Parallel Reduction with "&&" operator does not compute correct result

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94366

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.