This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, RFC?] Verify that VCE(SSA_NAME) does not occur on any LHS


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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]