[2/3] expmed: Fix possible use of NULL_RTX return value from emit_store_flag

Message ID 20200721182649.mrhv4mk2qgigs7tv@jozef-acer-manjaro
State New
Headers show
Series
  • MSP430: Improve code-generation for shift instructions
Related show

Commit Message

Jozef Lawrynowicz July 21, 2020, 6:26 p.m.
MSP430 does not support have any store-flag instructions, so
emit_store_flag can return NULL_RTX.  Catch the NULL_RTX in
expmed.c:expand_sdiv_pow2.

Successfully regtested on trunk for x86_64-pc-linux-gnu and msp430-elf.
Ok to apply?
From cb0f8219ac90229c0336281d73f511c107c877d3 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>

Date: Wed, 15 Jul 2020 11:19:31 +0100
Subject: [PATCH 2/3] expmed: Fix possible use of NULL_RTX return value from
 emit_store_flag

MSP430 does not support have any store-flag instructions, so
emit_store_flag can return NULL_RTX.  Catch the NULL_RTX in
expmed.c:expand_sdiv_pow2.

gcc/ChangeLog:

	* expmed.c (expand_sdiv_pow2): Check return value from emit_store_flag
	is not NULL_RTX before use.

---
 gcc/expmed.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

-- 
2.27.0

Comments

Richard Sandiford July 22, 2020, 8:38 a.m. | #1
Jozef Lawrynowicz <jozef.l@mittosystems.com> writes:
> diff --git a/gcc/expmed.c b/gcc/expmed.c

> index e7c03fbf92c..d3a1735d39e 100644

> --- a/gcc/expmed.c

> +++ b/gcc/expmed.c

> @@ -4086,9 +4086,12 @@ expand_sdiv_pow2 (scalar_int_mode mode, rtx op0, HOST_WIDE_INT d)

>      {

>        temp = gen_reg_rtx (mode);

>        temp = emit_store_flag (temp, LT, op0, const0_rtx, mode, 0, 1);

> -      temp = expand_binop (mode, add_optab, temp, op0, NULL_RTX,

> -			   0, OPTAB_LIB_WIDEN);

> -      return expand_shift (RSHIFT_EXPR, mode, temp, logd, NULL_RTX, 0);

> +      if (temp != NULL_RTX)

> +	{

> +	  temp = expand_binop (mode, add_optab, temp, op0, NULL_RTX,

> +			       0, OPTAB_LIB_WIDEN);

> +	  return expand_shift (RSHIFT_EXPR, mode, temp, logd, NULL_RTX, 0);

> +	}

>      }

>  

>    if (HAVE_conditional_move

> @@ -4122,17 +4125,20 @@ expand_sdiv_pow2 (scalar_int_mode mode, rtx op0, HOST_WIDE_INT d)

>  

>        temp = gen_reg_rtx (mode);

>        temp = emit_store_flag (temp, LT, op0, const0_rtx, mode, 0, -1);

> -      if (GET_MODE_BITSIZE (mode) >= BITS_PER_WORD

> -	  || shift_cost (optimize_insn_for_speed_p (), mode, ushift)

> -	     > COSTS_N_INSNS (1))

> -	temp = expand_binop (mode, and_optab, temp, gen_int_mode (d - 1, mode),

> -			     NULL_RTX, 0, OPTAB_LIB_WIDEN);

> -      else

> -	temp = expand_shift (RSHIFT_EXPR, mode, temp,

> -			     ushift, NULL_RTX, 1);

> -      temp = expand_binop (mode, add_optab, temp, op0, NULL_RTX,

> -			   0, OPTAB_LIB_WIDEN);

> -      return expand_shift (RSHIFT_EXPR, mode, temp, logd, NULL_RTX, 0);

> +      if (temp != NULL_RTX)

> +	{

> +	  if (GET_MODE_BITSIZE (mode) >= BITS_PER_WORD

> +	      || shift_cost (optimize_insn_for_speed_p (), mode, ushift)

> +	      > COSTS_N_INSNS (1))

> +	    temp = expand_binop (mode, and_optab, temp, gen_int_mode (d - 1, mode),


Long line.

OK otherwise, thanks.  I guess these failed attempts will leave
a few unused temporary registers around (from the gen_reg_rtxes)
but it's going to be hard to avoid that in a clean way.

Richard

> +				 NULL_RTX, 0, OPTAB_LIB_WIDEN);

> +	  else

> +	    temp = expand_shift (RSHIFT_EXPR, mode, temp,

> +				 ushift, NULL_RTX, 1);

> +	  temp = expand_binop (mode, add_optab, temp, op0, NULL_RTX,

> +			       0, OPTAB_LIB_WIDEN);

> +	  return expand_shift (RSHIFT_EXPR, mode, temp, logd, NULL_RTX, 0);

> +	}

>      }

>  

>    label = gen_label_rtx ();

Patch

diff --git a/gcc/expmed.c b/gcc/expmed.c
index e7c03fbf92c..d3a1735d39e 100644
--- a/gcc/expmed.c
+++ b/gcc/expmed.c
@@ -4086,9 +4086,12 @@  expand_sdiv_pow2 (scalar_int_mode mode, rtx op0, HOST_WIDE_INT d)
     {
       temp = gen_reg_rtx (mode);
       temp = emit_store_flag (temp, LT, op0, const0_rtx, mode, 0, 1);
-      temp = expand_binop (mode, add_optab, temp, op0, NULL_RTX,
-			   0, OPTAB_LIB_WIDEN);
-      return expand_shift (RSHIFT_EXPR, mode, temp, logd, NULL_RTX, 0);
+      if (temp != NULL_RTX)
+	{
+	  temp = expand_binop (mode, add_optab, temp, op0, NULL_RTX,
+			       0, OPTAB_LIB_WIDEN);
+	  return expand_shift (RSHIFT_EXPR, mode, temp, logd, NULL_RTX, 0);
+	}
     }
 
   if (HAVE_conditional_move
@@ -4122,17 +4125,20 @@  expand_sdiv_pow2 (scalar_int_mode mode, rtx op0, HOST_WIDE_INT d)
 
       temp = gen_reg_rtx (mode);
       temp = emit_store_flag (temp, LT, op0, const0_rtx, mode, 0, -1);
-      if (GET_MODE_BITSIZE (mode) >= BITS_PER_WORD
-	  || shift_cost (optimize_insn_for_speed_p (), mode, ushift)
-	     > COSTS_N_INSNS (1))
-	temp = expand_binop (mode, and_optab, temp, gen_int_mode (d - 1, mode),
-			     NULL_RTX, 0, OPTAB_LIB_WIDEN);
-      else
-	temp = expand_shift (RSHIFT_EXPR, mode, temp,
-			     ushift, NULL_RTX, 1);
-      temp = expand_binop (mode, add_optab, temp, op0, NULL_RTX,
-			   0, OPTAB_LIB_WIDEN);
-      return expand_shift (RSHIFT_EXPR, mode, temp, logd, NULL_RTX, 0);
+      if (temp != NULL_RTX)
+	{
+	  if (GET_MODE_BITSIZE (mode) >= BITS_PER_WORD
+	      || shift_cost (optimize_insn_for_speed_p (), mode, ushift)
+	      > COSTS_N_INSNS (1))
+	    temp = expand_binop (mode, and_optab, temp, gen_int_mode (d - 1, mode),
+				 NULL_RTX, 0, OPTAB_LIB_WIDEN);
+	  else
+	    temp = expand_shift (RSHIFT_EXPR, mode, temp,
+				 ushift, NULL_RTX, 1);
+	  temp = expand_binop (mode, add_optab, temp, op0, NULL_RTX,
+			       0, OPTAB_LIB_WIDEN);
+	  return expand_shift (RSHIFT_EXPR, mode, temp, logd, NULL_RTX, 0);
+	}
     }
 
   label = gen_label_rtx ();