diff mbox series

recog: Fix propagation into ASM_OPERANDS

Message ID mptlebspbrf.fsf@arm.com
State New
Headers show
Series recog: Fix propagation into ASM_OPERANDS | expand

Commit Message

Richard Sandiford Oct. 24, 2023, 10:15 a.m. UTC 
An inline asm with multiple output operands is represented as a
parallel set in which the SET_SRCs are the same (shared) ASM_OPERANDS.
insn_propgation didn't account for this, and instead propagated
into each ASM_OPERANDS individually.  This meant that it could
apply a substitution X->Y to Y itself, which (a) could create
circularity and (b) would be semantically wrong in any case,
since Y might use a different value of X.

This patch checks explicitly for parallels involving ASM_OPERANDS,
just like combine does.

Tested on aarch64-linux-gnu & x86_64-linux-gnu.  OK to install?

Richard


gcc/
	* recog.cc (insn_propagation::apply_to_pattern_1): Handle shared
	ASM_OPERANDS.
---
 gcc/recog.cc | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

Comments

Jeff Law Oct. 27, 2023, 2:48 p.m. UTC | #1 
On 10/24/23 04:15, Richard Sandiford wrote:
> An inline asm with multiple output operands is represented as a
> parallel set in which the SET_SRCs are the same (shared) ASM_OPERANDS.
> insn_propgation didn't account for this, and instead propagated
> into each ASM_OPERANDS individually.  This meant that it could
> apply a substitution X->Y to Y itself, which (a) could create
> circularity and (b) would be semantically wrong in any case,
> since Y might use a different value of X.
> 
> This patch checks explicitly for parallels involving ASM_OPERANDS,
> just like combine does.
> 
> Tested on aarch64-linux-gnu & x86_64-linux-gnu.  OK to install?
> 
> Richard
> 
> 
> gcc/
> 	* recog.cc (insn_propagation::apply_to_pattern_1): Handle shared
> 	ASM_OPERANDS.
As the combine comment says "Ug".  OK for the trunk.

jeff
Richard Sandiford Oct. 27, 2023, 3:38 p.m. UTC | #2 
Jeff Law <jeffreyalaw@gmail.com> writes:
> On 10/24/23 04:15, Richard Sandiford wrote:
>> An inline asm with multiple output operands is represented as a
>> parallel set in which the SET_SRCs are the same (shared) ASM_OPERANDS.
>> insn_propgation didn't account for this, and instead propagated
>> into each ASM_OPERANDS individually.  This meant that it could
>> apply a substitution X->Y to Y itself, which (a) could create
>> circularity and (b) would be semantically wrong in any case,
>> since Y might use a different value of X.
>> 
>> This patch checks explicitly for parallels involving ASM_OPERANDS,
>> just like combine does.
>> 
>> Tested on aarch64-linux-gnu & x86_64-linux-gnu.  OK to install?
>> 
>> Richard
>> 
>> 
>> gcc/
>> 	* recog.cc (insn_propagation::apply_to_pattern_1): Handle shared
>> 	ASM_OPERANDS.
> As the combine comment says "Ug".

Aye :)  Thanks for the reviews.

Richard

>  OK for the trunk.
>
> jeff
diff mbox series

Patch

diff --git a/gcc/recog.cc b/gcc/recog.cc
index e12b4c9500e..3bd2d73c259 100644
--- a/gcc/recog.cc
+++ b/gcc/recog.cc
@@ -1339,13 +1339,26 @@  insn_propagation::apply_to_pattern_1 (rtx *loc)
 	      && apply_to_pattern_1 (&COND_EXEC_CODE (body)));
 
     case PARALLEL:
-      {
-	int last = XVECLEN (body, 0) - 1;
-	for (int i = 0; i < last; ++i)
-	  if (!apply_to_pattern_1 (&XVECEXP (body, 0, i)))
-	    return false;
-	return apply_to_pattern_1 (&XVECEXP (body, 0, last));
-      }
+      for (int i = 0; i < XVECLEN (body, 0); ++i)
+	{
+	  rtx *subloc = &XVECEXP (body, 0, i);
+	  if (GET_CODE (*subloc) == SET)
+	    {
+	      if (!apply_to_lvalue_1 (SET_DEST (*subloc)))
+		return false;
+	      /* ASM_OPERANDS are shared between SETs in the same PARALLEL.
+		 Only process them on the first iteration.  */
+	      if ((i == 0 || GET_CODE (SET_SRC (*subloc)) != ASM_OPERANDS)
+		  && !apply_to_rvalue_1 (&SET_SRC (*subloc)))
+		return false;
+	    }
+	  else
+	    {
+	      if (!apply_to_pattern_1 (subloc))
+		return false;
+	    }
+	}
+      return true;
 
     case ASM_OPERANDS:
       for (int i = 0, len = ASM_OPERANDS_INPUT_LENGTH (body); i < len; ++i)