C++ PATCH to reduced_constant_expression for partially-initialized objects

Message ID CADzB+2nYg5K3Br20V=HxSuMYXKOpE91r3A7moQfeMvo9wreR+w@mail.gmail.com
State New
Headers show
Series
  • C++ PATCH to reduced_constant_expression for partially-initialized objects
Related show

Commit Message

Jason Merrill Sept. 10, 2017, 9:09 a.m.
A few months back I queued this patch to bring back for GCC 8.
Unfortunately I don't remember the context that it came up in, but it
affects for instance cases of self-assignment, which can't have a
constant value if there is no previous initialization.

Tested x86_64-pc-linux-gnu, applying to trunk.
commit 8c5592e0e31ec60bb6b075fca14face587e3084b
Author: Jason Merrill <jason@redhat.com>
Date:   Tue Apr 11 16:29:37 2017 -0400

            * constexpr.c (reduced_constant_expression_p): If
            CONSTRUCTOR_NO_IMPLICIT_ZERO, check that all fields are initialized.

Comments

Christophe Lyon Sept. 12, 2017, 2:17 p.m. | #1
Hi Jason

On 10 September 2017 at 11:09, Jason Merrill <jason@redhat.com> wrote:
> A few months back I queued this patch to bring back for GCC 8.
> Unfortunately I don't remember the context that it came up in, but it
> affects for instance cases of self-assignment, which can't have a
> constant value if there is no previous initialization.
>
> Tested x86_64-pc-linux-gnu, applying to trunk.

I've noticed that this patch causes a regression in fortran on
armeb-linux-gnumeabihf
FAIL:    gfortran.dg/allocate_zerosize_3.f   -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions  execution
test
FAIL:    gfortran.dg/allocate_zerosize_3.f   -O3 -g  execution test
FAIL:    gfortran.dg/assumed_rank_1.f90   -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions  execution
test
FAIL:    gfortran.dg/assumed_rank_1.f90   -O3 -g  execution test
FAIL:    gfortran.dg/assumed_rank_2.f90   -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions  execution
test
FAIL:    gfortran.dg/assumed_rank_2.f90   -O3 -g  execution test

target armeb-none-linux-gnueabihf
--with-mode arm
--with-cpu cortex-a9
--with-fpu neon-fp16

using --with-fpu vfpv3-d16-fp16 does not introduce the regression.

target arm-none-linux-gnueabihf with the same parameters is OK ('arm'
as opposed to 'armeb')

My gfortran.log only shows:
spawn [open ...]
Program aborted. Backtrace:
qemu: uncaught target signal 6 (Aborted) - core dumped

Christophe
Andrew Pinski Sept. 12, 2017, 2:21 p.m. | #2
On Tue, Sep 12, 2017 at 7:17 AM, Christophe Lyon
<christophe.lyon@linaro.org> wrote:
> Hi Jason
>
> On 10 September 2017 at 11:09, Jason Merrill <jason@redhat.com> wrote:
>> A few months back I queued this patch to bring back for GCC 8.
>> Unfortunately I don't remember the context that it came up in, but it
>> affects for instance cases of self-assignment, which can't have a
>> constant value if there is no previous initialization.
>>
>> Tested x86_64-pc-linux-gnu, applying to trunk.
>
> I've noticed that this patch causes a regression in fortran on
> armeb-linux-gnumeabihf
> FAIL:    gfortran.dg/allocate_zerosize_3.f   -O3 -fomit-frame-pointer
> -funroll-loops -fpeel-loops -ftracer -finline-functions  execution
> test
> FAIL:    gfortran.dg/allocate_zerosize_3.f   -O3 -g  execution test
> FAIL:    gfortran.dg/assumed_rank_1.f90   -O3 -fomit-frame-pointer
> -funroll-loops -fpeel-loops -ftracer -finline-functions  execution
> test
> FAIL:    gfortran.dg/assumed_rank_1.f90   -O3 -g  execution test
> FAIL:    gfortran.dg/assumed_rank_2.f90   -O3 -fomit-frame-pointer
> -funroll-loops -fpeel-loops -ftracer -finline-functions  execution
> test
> FAIL:    gfortran.dg/assumed_rank_2.f90   -O3 -g  execution test
>
> target armeb-none-linux-gnueabihf
> --with-mode arm
> --with-cpu cortex-a9
> --with-fpu neon-fp16
>
> using --with-fpu vfpv3-d16-fp16 does not introduce the regression.
>
> target arm-none-linux-gnueabihf with the same parameters is OK ('arm'
> as opposed to 'armeb')
>
> My gfortran.log only shows:
> spawn [open ...]
> Program aborted. Backtrace:
> qemu: uncaught target signal 6 (Aborted) - core dumped

This makes little sense.  Are you compiling the cross compiler with
the new native compiler?  Since this patch only touches the C++
front-end and only C++11 even that makes less sense.  Are you sure
this was not a bug in qemu which is just happening showing up now?
Even then this makes little sense as the code generation between the
two revisions should not touch anything related to fortran.

Thanks,
Andrew Pinski

>
> Christophe
Christophe Lyon Sept. 12, 2017, 2:52 p.m. | #3
On 12 September 2017 at 16:21, Andrew Pinski <pinskia@gmail.com> wrote:
> On Tue, Sep 12, 2017 at 7:17 AM, Christophe Lyon
> <christophe.lyon@linaro.org> wrote:
>> Hi Jason
>>
>> On 10 September 2017 at 11:09, Jason Merrill <jason@redhat.com> wrote:
>>> A few months back I queued this patch to bring back for GCC 8.
>>> Unfortunately I don't remember the context that it came up in, but it
>>> affects for instance cases of self-assignment, which can't have a
>>> constant value if there is no previous initialization.
>>>
>>> Tested x86_64-pc-linux-gnu, applying to trunk.
>>
>> I've noticed that this patch causes a regression in fortran on
>> armeb-linux-gnumeabihf
>> FAIL:    gfortran.dg/allocate_zerosize_3.f   -O3 -fomit-frame-pointer
>> -funroll-loops -fpeel-loops -ftracer -finline-functions  execution
>> test
>> FAIL:    gfortran.dg/allocate_zerosize_3.f   -O3 -g  execution test
>> FAIL:    gfortran.dg/assumed_rank_1.f90   -O3 -fomit-frame-pointer
>> -funroll-loops -fpeel-loops -ftracer -finline-functions  execution
>> test
>> FAIL:    gfortran.dg/assumed_rank_1.f90   -O3 -g  execution test
>> FAIL:    gfortran.dg/assumed_rank_2.f90   -O3 -fomit-frame-pointer
>> -funroll-loops -fpeel-loops -ftracer -finline-functions  execution
>> test
>> FAIL:    gfortran.dg/assumed_rank_2.f90   -O3 -g  execution test
>>
>> target armeb-none-linux-gnueabihf
>> --with-mode arm
>> --with-cpu cortex-a9
>> --with-fpu neon-fp16
>>
>> using --with-fpu vfpv3-d16-fp16 does not introduce the regression.
>>
>> target arm-none-linux-gnueabihf with the same parameters is OK ('arm'
>> as opposed to 'armeb')
>>
>> My gfortran.log only shows:
>> spawn [open ...]
>> Program aborted. Backtrace:
>> qemu: uncaught target signal 6 (Aborted) - core dumped
>
> This makes little sense.  Are you compiling the cross compiler with
> the new native compiler?  Since this patch only touches the C++
> front-end and only C++11 even that makes less sense.  Are you sure
> this was not a bug in qemu which is just happening showing up now?
> Even then this makes little sense as the code generation between the
> two revisions should not touch anything related to fortran.
>

Hmmmm you are probably right. I'm compiling the cross compiler
with the system's native compiler.

Looks like I need to check why my bisect script returns the wrong revision.

Thanks,

Christophe

> Thanks,
> Andrew Pinski
>
>>
>> Christophe

Patch

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index a5692fb..2d2f3b8 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -1732,15 +1732,30 @@  reduced_constant_expression_p (tree t)
 
     case CONSTRUCTOR:
       /* And we need to handle PTRMEM_CST wrapped in a CONSTRUCTOR.  */
-      tree elt; unsigned HOST_WIDE_INT idx;
-      FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (t), idx, elt)
+      tree idx, val, field; unsigned HOST_WIDE_INT i;
+      if (CONSTRUCTOR_NO_IMPLICIT_ZERO (t))
+	field = next_initializable_field (TYPE_FIELDS (TREE_TYPE (t)));
+      else
+	field = NULL_TREE;
+      FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (t), i, idx, val)
 	{
-	  if (!elt)
+	  if (!val)
 	    /* We're in the middle of initializing this element.  */
 	    return false;
-	  if (!reduced_constant_expression_p (elt))
+	  if (!reduced_constant_expression_p (val))
 	    return false;
+	  if (field)
+	    {
+	      if (idx != field)
+		return false;
+	      field = next_initializable_field (DECL_CHAIN (field));
+	    }
 	}
+      if (field)
+	return false;
+      else if (CONSTRUCTOR_NO_IMPLICIT_ZERO (t))
+	/* All the fields are initialized.  */
+	CONSTRUCTOR_NO_IMPLICIT_ZERO (t) = false;
       return true;
 
     default: