On 9/30/21 11:42, Qing Zhao wrote:
On Sep 30, 2021, at 1:54 AM, Richard Biener <rguent...@suse.de> wrote:
On Thu, 30 Sep 2021, Jason Merrill wrote:
On 9/29/21 17:30, Qing Zhao wrote:
Hi,
PR102359 (ICE gimplification failed since r12-3433-ga25e0b5e6ac8a77a)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102359
Is due to -ftrivial-auto-var-init adding initialization for READONLY
variable “this” in the following routine: (t.cpp.005t.original)
=======
;; Function A::foo()::<lambda()> (null)
;; enabled by -tree-original
{
const struct A * const this [value-expr: &__closure->__this];
const struct A * const this [value-expr: &__closure->__this];
return <retval> = (double) ((const struct A *) this)->a;
}
=======
However, in the above routine, “this” is NOT marked as READONLY, but its
value-expr "&__closure->__this” is marked as READONLY.
There are two major issues:
1. In the routine “is_var_need_auto_init”, we should exclude “decl” that is
marked as READONLY;
2. In the C++ FE, “this” should be marked as READONLY.
The idea solution will be:
1. Fix “is_var_need_auto_init” to exclude TREE_READONLY (decl);
2. Fix C++ FE to mark “this” as TREE_READONLY (decl)==true;
Not sure whether it’s hard for C++ FE to fix the 2nd issue or not?
In the case it’s not a quick fix in C++FE, I proposed the following fix in
middle end:
Let me know your comments or suggestions on this.
Thanks a lot for the help.
I'd think is_var_need_auto_init should be false for any variable with
DECL_HAS_VALUE_EXPR_P, as they aren't really variables, just ways of naming
objects that are initialized elsewhere.
IIRC handing variables with DECL_HAS_VALUE_EXPR_P is necessary to
auto-init VLAs, otherwise I tend to agree - would we handle those
when we see a DECL_EXPR then?
The current implementation is:
gimplify_decl_expr:
For each DECL_EXPR “decl”
If (VAR_P (decl) && !DECL_EXTERNAL (decl))
{
if (is_vla (decl))
gimplify_vla_decl (decl, …); /* existing handling: create a
VALUE_EXPR for this vla decl*/
…
if (has_explicit_init (decl))
{
…; /* existing handling. */
}
else if (is_var_need_auto_init (decl)) /*. New code. */
{
gimple_add_init_for_auto_var (….); /* new code. */
...
}
}
Since the “DECL_VALUE_EXPR (decl)” is NOT a DECL_EXPR, it will not be scanned
and added initialization.
if we do not add initialization for a decl that has DECL_VALUE_EXPR, then the
“DECL_VALUE_EXPR (decl)” will not be added an initialization either. We will
miss adding initializations for these decls.
So, I think that the current implementation is correct.
And if C++ FE will not mark “this” as READONLY, only mark DECL_VALUE_EXPR(this)
as READONLY, the proposed fix is correct too.
Let me know your opinion on this.
The problem with this test is not whether the 'this' proxy is marked
READONLY, the problem is that you're trying to initialize lambda capture
proxies at all; the lambda capture objects were already initialized when
forming the closure object. So this test currently aborts with
-ftrivial-auto-var-init=zero because you "initialize" the i capture
field to 0 after it was previously initialized to 42:
int main()
{
int i = 42;
auto l = [=]() mutable { return i; };
if (l() != i)
__builtin_abort ();
}
I believe the same issue applies to the proxy variables in coroutines
that work much like lambdas.
You can't just assume that a VAR_DECL with DECL_VALUE_EXPR is uninitialized.
Since there's already VLA handling in gimplify_decl_expr, you could
remember whether you added DECL_VALUE_EXPR in that function, and only
then do the initialization.
Jason
