diff mbox

Fix ICE on unaligned record field

Message ID 3887233.jQSJCmu5YU@polaris
State New
Headers show

Commit Message

Eric Botcazou Nov. 28, 2014, 4:20 p.m. UTC 
Hi,

the attached Ada testcase triggers an assertion in the RTL expander for the 
address operator because the operator has been applied to a non-byte-aligned  
record field.  The problematic ADDR_EXPR is built by ipa_modify_call_arguments 
which has a hole when get_addr_base_and_unit_offset returns NULL_TREE: the 
variable offset case is handled but not the non-byte-aligned case, which can 
rountinely happen in Ada, hence the proposed fix.

Tested on x86_64-suse-linux, OK for the mainline?


2014-11-28  Eric Botcazou  <ebotcazou@adacore.com>

	* ipa-prop.c (ipa_modify_call_arguments): Properly deal with unaligned
	aggregate parameters passed by value.


2014-11-28  Eric Botcazou  <ebotcazou@adacore.com>

	* gnat.dg/specs/pack12.ads: New test.

Comments

Richard Biener Dec. 1, 2014, 11 a.m. UTC | #1 
On Fri, Nov 28, 2014 at 5:20 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> the attached Ada testcase triggers an assertion in the RTL expander for the
> address operator because the operator has been applied to a non-byte-aligned
> record field.  The problematic ADDR_EXPR is built by ipa_modify_call_arguments
> which has a hole when get_addr_base_and_unit_offset returns NULL_TREE: the
> variable offset case is handled but not the non-byte-aligned case, which can
> rountinely happen in Ada, hence the proposed fix.
>
> Tested on x86_64-suse-linux, OK for the mainline?

Umm.  So you are doing a possibly aggregate copy here?  Or how
are we sure we are dealing with a register op only?  (the function
is always a twisted maze to me...)

That said - I suppose this is called from IPA-SRA?  In that case,
can't we please avoid doing the transform in the first place?

Thanks,
Richard.

>
> 2014-11-28  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * ipa-prop.c (ipa_modify_call_arguments): Properly deal with unaligned
>         aggregate parameters passed by value.
>
>
> 2014-11-28  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * gnat.dg/specs/pack12.ads: New test.
>
>
> --
> Eric Botcazou
diff mbox

Patch

Index: ipa-prop.c
===================================================================
--- ipa-prop.c	(revision 218133)
+++ ipa-prop.c	(working copy)
@@ -3939,6 +3939,39 @@  ipa_modify_call_arguments (struct cgraph
 	      /* Aggregate arguments can have non-invariant addresses.  */
 	      if (!base)
 		{
+		  /* ??? If the original aggregate is passed by value, it may
+		     be not aligned on a unit boundary, in which case taking
+		     directly its address below is forbidden.  Unfortunately
+		     get_addr_base_and_unit_offset doesn't differentiate its
+		     two failure modes so we need to get our hands dirty.  */
+		  if (!addrof)
+		    {
+		      tree offset;
+		      HOST_WIDE_INT bitsize, bitpos;
+		      machine_mode mode;
+		      int unsignedp, volatilep = 0;
+		      get_inner_reference (prev_base, &bitsize, &bitpos,
+					   &offset, &mode, &unsignedp,
+					   &volatilep, false);
+		      if (bitpos % BITS_PER_UNIT)
+			{
+			  tree tmp
+			    = create_tmp_reg (TREE_TYPE (prev_base), NULL);
+			  tree *argp
+			    = gimple_call_arg_ptr (stmt, adj->base_index);
+			  gimple tem = gimple_build_assign (tmp, prev_base);
+			  tree vuse = gimple_vuse (stmt);
+			  tree new_vdef = copy_ssa_name (vuse, tem);
+			  gimple_set_vuse (tem, vuse);
+			  gimple_set_vdef (tem, new_vdef);
+			  SET_USE (gimple_vuse_op (stmt), new_vdef);
+			  /* Insert the temporary ahead of every subsequent
+			     adjustment and replace the argument in the call
+			     in case it is referenced more than once.  */
+			  gsi_insert_after (&prev_gsi, tem, GSI_SAME_STMT);
+			  *argp = prev_base = tmp;
+			}
+		    }
 		  base = build_fold_addr_expr (prev_base);
 		  off = build_int_cst (adj->alias_ptr_type,
 				       adj->offset / BITS_PER_UNIT);