This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, RFC?] Verify that VCE(SSA_NAME) does not occur on any LHS
- From: Richard Guenther <rguenther at suse dot de>
- To: Martin Jambor <mjambor at suse dot cz>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 18 Nov 2009 14:00:37 +0100 (CET)
- Subject: Re: [PATCH, RFC?] Verify that VCE(SSA_NAME) does not occur on any LHS
- References: <20091118125233.GA10170@virgil.suse.cz>
On Wed, 18 Nov 2009, Martin Jambor wrote:
> Hi,
>
> when hunting down some gimple grammar problems in IL that IPA-SRA
> produces, I stumbled upon the fact that VIEW_CONVERT_EXPRs of
> SSA_NAMEs on the left hand sides of gimple assign and call statements
> are not caught by the statement verifier. So I added a check for that
> to verify_types_in_gimple_reference which errors out for example when
> fed the testcase in comment 5 of PR 42025 (on x86_64).
>
> I have also changed verify_gimple_call to check the LHS with
> verify_types_in_gimple_reference rather than with mere
> is_gimple_lvalue. I have looked carefully at the code and believe
> that it is the correct thing to do but I would not be very surprised
> if someone proved me wrong.
>
> I have also bootstrapped and tested the patch below on x86_64-linux
> with no regressions. I would certainly like to commit a patch like
> this when stage1 opens again. I am not sure whether it is a good idea
> to commit it now (now meaning after I fix the PR) during stage3 but
> will gladly do it if a release manager agrees. Nevertheless, I will
> use it to test my IPA-SRA fixes right now and so am interested in any
> feedback.
Below...
> Index: mine/gcc/tree-cfg.c
> ===================================================================
> --- mine.orig/gcc/tree-cfg.c
> +++ mine/gcc/tree-cfg.c
> @@ -2889,12 +2889,22 @@ verify_types_in_gimple_reference (tree e
> return true;
> }
>
> - /* For VIEW_CONVERT_EXPRs which are allowed here, too, there
> - is nothing to verify. Gross mismatches at most invoke
> - undefined behavior. */
> - if (TREE_CODE (expr) == VIEW_CONVERT_EXPR
> - && !handled_component_p (op))
> - return false;
> + if (TREE_CODE (expr) == VIEW_CONVERT_EXPR)
> + {
> + /* For VIEW_CONVERT_EXPRs which are allowed here too, we only check
> + that their operand is not an SSA name when requiring an lvalue
> + (this usually means there is a SRA or IPA-SRA bug). Otherwise
> + there is nothing to verify, gross mismatches at most invoke
> + undefined behavior. */
> + if (require_lvalue && TREE_CODE (op) == SSA_NAME)
This should also exclude is_gimple_min_invariant (op), thus constants
on the LHS. Like
if (require_lvalue
&& (TREE_CODE (op) == SSA_NAME
|| is_gimple_min_invariant (op)))
> + {
> + error ("Conversion of an SSA_NAME on the left hand side.");
> + debug_generic_stmt (expr);
> + return true;
> + }
> + else if (!handled_component_p (op))
> + return false;
> + }
>
> expr = op;
> }
> @@ -2951,7 +2961,7 @@ verify_gimple_call (gimple stmt)
> }
>
> if (gimple_call_lhs (stmt)
> - && !is_gimple_lvalue (gimple_call_lhs (stmt)))
> + && verify_types_in_gimple_reference (gimple_call_lhs (stmt), true))
Even while (somewhat) redundant please keep the is_gimple_lvalue check,
like
&& (!is_gimple_lvalue (gimple_call_lhs (stmt)))
|| verify_types_in_gimple_reference (gimple_call_lhs (stmt), true))
Ok with that changes.
Thanks,
Richard.