Patchwork [mips] Fix for PR 54619, GCC aborting with -O -mips16

login
register
mail settings
Submitter Tom de Vries
Date Nov. 14, 2012, 10:29 p.m.
Message ID <50A41B50.5030907@mentor.com>
Download mbox | patch
Permalink /patch/199041/
State New
Headers show

Comments

Tom de Vries - Nov. 14, 2012, 10:29 p.m.
On 21/09/12 03:40, Sandra Loosemore wrote:
> Re:
> 
>> I think tree-ssa-loop-ivopts is simply
>> asking for the wrong thing, and needs to be changed.  As I say,
>> Sandra had some fixes in this area.
> 
> This patch:
> 
> http://gcc.gnu.org/ml/gcc-patches/2012-06/msg00319.html
> 
> Sadly, that patch has fallen off the bottom of my priority list (some 
> legal wrangling halted all work on the Hexagon port that was the 
> motivating example for it), and meanwhile it's become bit-rotten due to 
> Bill Schmidt's work that touched on the ivopts code as well.  If someone 
> else would like to "adopt" this patch, please do!
> 
> I agree that it is just plain dumb for ivopts to be trying to precompute 
> address costs for modes that are not even valid.
> 

Zdenek,

this patch prevents ivopts from passing VOIDmode to address_cost.

The patch fixes a buildbreaker in newlib for mips16. I've confirmed that I can
do a full mips build with newlib with this patch.

I've bootstrapped and reg-tested this patch on x86_64.

OK for trunk (without the assert)?

Thanks,
- Tom

2012-11-14  Tom de Vries  <tom@codesourcery.com>

	* tree-ssa-loop-ivopts.c (get_use_type): New function.
	(get_computation_at): Use get_use_type.
	(get_computation_cost_at): Declare and set mem_mode.  Use mem_mode.
Zdenek Dvorak - Nov. 15, 2012, 1:39 p.m.
Hi,

> >> I think tree-ssa-loop-ivopts is simply
> >> asking for the wrong thing, and needs to be changed.  As I say,
> >> Sandra had some fixes in this area.
> > 
> > This patch:
> > 
> > http://gcc.gnu.org/ml/gcc-patches/2012-06/msg00319.html
> > 
> > Sadly, that patch has fallen off the bottom of my priority list (some 
> > legal wrangling halted all work on the Hexagon port that was the 
> > motivating example for it), and meanwhile it's become bit-rotten due to 
> > Bill Schmidt's work that touched on the ivopts code as well.  If someone 
> > else would like to "adopt" this patch, please do!
> > 
> > I agree that it is just plain dumb for ivopts to be trying to precompute 
> > address costs for modes that are not even valid.
> > 
> 
> Zdenek,
> 
> this patch prevents ivopts from passing VOIDmode to address_cost.
> 
> The patch fixes a buildbreaker in newlib for mips16. I've confirmed that I can
> do a full mips build with newlib with this patch.
> 
> I've bootstrapped and reg-tested this patch on x86_64.
> 
> OK for trunk (without the assert)?
> 
> Thanks,
> - Tom
> 
> 2012-11-14  Tom de Vries  <tom@codesourcery.com>
> 
> 	* tree-ssa-loop-ivopts.c (get_use_type): New function.
> 	(get_computation_at): Use get_use_type.
> 	(get_computation_cost_at): Declare and set mem_mode.  Use mem_mode.

OK,

Zdenek
Richard Sandiford - Nov. 19, 2012, 12:01 p.m.
Tom de Vries <Tom_deVries@mentor.com> writes:
> On 21/09/12 03:40, Sandra Loosemore wrote:
>> Re:
>> 
>>> I think tree-ssa-loop-ivopts is simply
>>> asking for the wrong thing, and needs to be changed.  As I say,
>>> Sandra had some fixes in this area.
>> 
>> This patch:
>> 
>> http://gcc.gnu.org/ml/gcc-patches/2012-06/msg00319.html
>> 
>> Sadly, that patch has fallen off the bottom of my priority list (some 
>> legal wrangling halted all work on the Hexagon port that was the 
>> motivating example for it), and meanwhile it's become bit-rotten due to 
>> Bill Schmidt's work that touched on the ivopts code as well.  If someone 
>> else would like to "adopt" this patch, please do!
>> 
>> I agree that it is just plain dumb for ivopts to be trying to precompute 
>> address costs for modes that are not even valid.
>> 
>
> Zdenek,
>
> this patch prevents ivopts from passing VOIDmode to address_cost.
>
> The patch fixes a buildbreaker in newlib for mips16. I've confirmed that I can
> do a full mips build with newlib with this patch.

Many (belated) thanks for doing this.  It kept falling off my radar...

Richard

Patch

Index: gcc/rtlanal.c
===================================================================
--- gcc/rtlanal.c (revision 193478)
+++ gcc/rtlanal.c (working copy)
@@ -3840,6 +3840,8 @@  address_cost (rtx x, enum machine_mode m
      of push instruction.  It is not worthwhile to complicate writing
      of the target hook by such cases.  */
 
+  gcc_assert (mode != VOIDmode);
+
   if (!memory_address_addr_space_p (mode, x, as))
     return 1000;
 
Index: gcc/tree-ssa-loop-ivopts.c
===================================================================
--- gcc/tree-ssa-loop-ivopts.c (revision 193478)
+++ gcc/tree-ssa-loop-ivopts.c (working copy)
@@ -3009,6 +3009,28 @@  get_computation_aff (struct loop *loop,
   return true;
 }
 
+/* Return the type of USE.  */
+
+static tree
+get_use_type (struct iv_use *use)
+{
+  tree base_type = TREE_TYPE (use->iv->base);
+  tree type;
+
+  if (use->type == USE_ADDRESS)
+    {
+      /* The base_type may be a void pointer.  Create a pointer type based on
+	 the mem_ref instead.  */
+      type = build_pointer_type (TREE_TYPE (*use->op_p));
+      gcc_assert (TYPE_ADDR_SPACE (TREE_TYPE (type))
+		  == TYPE_ADDR_SPACE (TREE_TYPE (base_type)));
+    }
+  else
+    type = base_type;
+
+  return type;
+}
+
 /* Determines the expression by that USE is expressed from induction variable
    CAND at statement AT in LOOP.  The computation is unshared.  */
 
@@ -3017,7 +3039,7 @@  get_computation_at (struct loop *loop,
 		    struct iv_use *use, struct iv_cand *cand, gimple at)
 {
   aff_tree aff;
-  tree type = TREE_TYPE (use->iv->base);
+  tree type = get_use_type (use);
 
   if (!get_computation_aff (loop, use, cand, at, &aff))
     return NULL_TREE;
@@ -3934,6 +3956,9 @@  get_computation_cost_at (struct ivopts_d
   comp_cost cost;
   double_int rat;
   bool speed = optimize_bb_for_speed_p (gimple_bb (at));
+  enum machine_mode mem_mode = (address_p
+				? TYPE_MODE (TREE_TYPE (*use->op_p))
+				: VOIDmode);
 
   *depends_on = NULL;
 
@@ -4041,7 +4066,7 @@  get_computation_cost_at (struct ivopts_d
   else if (address_p
 	   && !POINTER_TYPE_P (ctype)
 	   && multiplier_allowed_in_address_p
-		(ratio, TYPE_MODE (TREE_TYPE (utype)),
+		(ratio, mem_mode,
 			TYPE_ADDR_SPACE (TREE_TYPE (utype))))
     {
       cbase
@@ -4085,7 +4110,7 @@  get_computation_cost_at (struct ivopts_d
     return add_costs (cost,
 		      get_address_cost (symbol_present, var_present,
 					offset, ratio, cstepi,
-					TYPE_MODE (TREE_TYPE (utype)),
+					mem_mode,
 					TYPE_ADDR_SPACE (TREE_TYPE (utype)),
 					speed, stmt_is_after_inc,
 					can_autoinc));