diff mbox series

[3/3] rs6000: NFC no need copy_rtx in rs6000_emit_set_long_const and rs6000_emit_set_const

Message ID 20221201013619.196004-3-guojiufu@linux.ibm.com
State New
Headers show
Series [1/3] rs6000: NFC use more readable pattern to clean high 32 bits | expand

Commit Message

Jiufu Guo Dec. 1, 2022, 1:36 a.m. UTC 
Hi,

Function rs6000_emit_set_const/rs6000_emit_set_long_const are only invoked from
two "define_split"s where the target operand is limited to gpc_reg_operand or
int_reg_operand, then the operand must be REG_P.
And in rs6000_emit_set_const/rs6000_emit_set_long_const, to create temp rtx,
it is using code like "gen_reg_rtx({S|D}Imode)", it must also be REG_P.
So, copy_rtx is not needed for temp and dest.

This patch removes those "copy_rtx" for rs6000_emit_set_const and
rs6000_emit_set_long_const.

Bootstrap & regtest pass on ppc64{,le}.
Is this ok for trunk? 

BR,
Jeff (Jiufu)

gcc/ChangeLog:

	* config/rs6000/rs6000.cc (rs6000_emit_set_const): Remove copy_rtx.
	(rs6000_emit_set_long_const): Likewise.

---
 gcc/config/rs6000/rs6000.cc | 58 +++++++++++++------------------------
 1 file changed, 20 insertions(+), 38 deletions(-)

Comments

Kewen.Lin Dec. 1, 2022, 3:31 a.m. UTC | #1 
Hi Jeff,

on 2022/12/1 09:36, Jiufu Guo wrote:
> Hi,
> 
> Function rs6000_emit_set_const/rs6000_emit_set_long_const are only invoked from
> two "define_split"s where the target operand is limited to gpc_reg_operand or
> int_reg_operand, then the operand must be REG_P.
> And in rs6000_emit_set_const/rs6000_emit_set_long_const, to create temp rtx,
> it is using code like "gen_reg_rtx({S|D}Imode)", it must also be REG_P.
> So, copy_rtx is not needed for temp and dest.
> 
> This patch removes those "copy_rtx" for rs6000_emit_set_const and
> rs6000_emit_set_long_const.
> 
> Bootstrap & regtest pass on ppc64{,le}.
> Is this ok for trunk? 

This patch is okay, thanks!  For the subject, IMHO it's better to use something
like: "rs6000: Remove useless copy_rtx in rs6000_emit_set_{,long}_const".
I don't see NFC tag used much in GCC, though it's used a lot in llvm, but
anyway you can append (NFC)/[NFC] at the end if you like.  :)

BR,
Kewen
Jiufu Guo Dec. 1, 2022, 4:52 a.m. UTC | #2 
Hi Kewen,

在 12/1/22 11:31 AM, Kewen.Lin 写道:
> Hi Jeff,
> 
> on 2022/12/1 09:36, Jiufu Guo wrote:
>> Hi,
>>
>> Function rs6000_emit_set_const/rs6000_emit_set_long_const are only invoked from
>> two "define_split"s where the target operand is limited to gpc_reg_operand or
>> int_reg_operand, then the operand must be REG_P.
>> And in rs6000_emit_set_const/rs6000_emit_set_long_const, to create temp rtx,
>> it is using code like "gen_reg_rtx({S|D}Imode)", it must also be REG_P.
>> So, copy_rtx is not needed for temp and dest.
>>
>> This patch removes those "copy_rtx" for rs6000_emit_set_const and
>> rs6000_emit_set_long_const.
>>
>> Bootstrap & regtest pass on ppc64{,le}.
>> Is this ok for trunk? 
> 
> This patch is okay, thanks!  For the subject, IMHO it's better to use something
> like: "rs6000: Remove useless copy_rtx in rs6000_emit_set_{,long}_const".
> I don't see NFC tag used much in GCC, though it's used a lot in llvm, but
> anyway you can append (NFC)/[NFC] at the end if you like.  :)
> 

"rs6000: Remove useless copy_rtx in rs6000_emit_set_{,long}_const" is great!

Thanks for your review and suggestions!


BR,
Jeff (Jiufu)

> BR,
> Kewen
Jiufu Guo Dec. 9, 2022, 12:25 p.m. UTC | #3 
Jiufu Guo via Gcc-patches <gcc-patches@gcc.gnu.org> writes:

> Hi Kewen,
>
> 在 12/1/22 11:31 AM, Kewen.Lin 写道:
>> Hi Jeff,
>> 
>> on 2022/12/1 09:36, Jiufu Guo wrote:
>>> Hi,
>>>
>>> Function rs6000_emit_set_const/rs6000_emit_set_long_const are only invoked from
>>> two "define_split"s where the target operand is limited to gpc_reg_operand or
>>> int_reg_operand, then the operand must be REG_P.
>>> And in rs6000_emit_set_const/rs6000_emit_set_long_const, to create temp rtx,
>>> it is using code like "gen_reg_rtx({S|D}Imode)", it must also be REG_P.
>>> So, copy_rtx is not needed for temp and dest.
>>>
>>> This patch removes those "copy_rtx" for rs6000_emit_set_const and
>>> rs6000_emit_set_long_const.
>>>
>>> Bootstrap & regtest pass on ppc64{,le}.
>>> Is this ok for trunk? 
>> 
>> This patch is okay, thanks!  For the subject, IMHO it's better to use something
>> like: "rs6000: Remove useless copy_rtx in rs6000_emit_set_{,long}_const".
>> I don't see NFC tag used much in GCC, though it's used a lot in llvm, but
>> anyway you can append (NFC)/[NFC] at the end if you like.  :)
>> 
>
> "rs6000: Remove useless copy_rtx in rs6000_emit_set_{,long}_const" is great!
>
> Thanks for your review and suggestions!

Thanks for comments and review! And committed via r13-4583-g71b31d13757ae0.

BR,
Jeff (Jiufu)
>
>
> BR,
> Jeff (Jiufu)
>
>> BR,
>> Kewen
diff mbox series

Patch

diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
index 718072cc9a1..1a51b79ebfe 100644
--- a/gcc/config/rs6000/rs6000.cc
+++ b/gcc/config/rs6000/rs6000.cc
@@ -10186,10 +10186,9 @@  rs6000_emit_set_const (rtx dest, rtx source)
     case E_SImode:
       temp = !can_create_pseudo_p () ? dest : gen_reg_rtx (SImode);
 
-      emit_insn (gen_rtx_SET (copy_rtx (temp),
-			      GEN_INT (c & ~(HOST_WIDE_INT) 0xffff)));
+      emit_insn (gen_rtx_SET (temp, GEN_INT (c & ~(HOST_WIDE_INT) 0xffff)));
       emit_insn (gen_rtx_SET (dest,
-			      gen_rtx_IOR (SImode, copy_rtx (temp),
+			      gen_rtx_IOR (SImode, temp,
 					   GEN_INT (c & 0xffff))));
       break;
 
@@ -10198,10 +10197,8 @@  rs6000_emit_set_const (rtx dest, rtx source)
 	{
 	  rtx hi, lo;
 
-	  hi = operand_subword_force (copy_rtx (dest), WORDS_BIG_ENDIAN == 0,
-				      DImode);
-	  lo = operand_subword_force (dest, WORDS_BIG_ENDIAN != 0,
-				      DImode);
+	  hi = operand_subword_force (dest, WORDS_BIG_ENDIAN == 0, DImode);
+	  lo = operand_subword_force (dest, WORDS_BIG_ENDIAN != 0, DImode);
 	  emit_move_insn (hi, GEN_INT (c >> 32));
 	  c = sext_hwi (c, 32);
 	  emit_move_insn (lo, GEN_INT (c));
@@ -10249,23 +10246,19 @@  rs6000_emit_set_long_const (rtx dest, HOST_WIDE_INT c)
     {
       temp = !can_create_pseudo_p () ? dest : gen_reg_rtx (DImode);
 
-      emit_move_insn (ud1 != 0 ? copy_rtx (temp) : dest,
+      emit_move_insn (ud1 != 0 ? temp : dest,
 		      GEN_INT (sext_hwi (ud2 << 16, 32)));
       if (ud1 != 0)
-	emit_move_insn (dest,
-			gen_rtx_IOR (DImode, copy_rtx (temp),
-				     GEN_INT (ud1)));
+	emit_move_insn (dest, gen_rtx_IOR (DImode, temp, GEN_INT (ud1)));
     }
   else if (ud3 == 0 && ud4 == 0)
     {
       temp = !can_create_pseudo_p () ? dest : gen_reg_rtx (DImode);
 
       gcc_assert (ud2 & 0x8000);
-      emit_move_insn (copy_rtx (temp), GEN_INT (sext_hwi (ud2 << 16, 32)));
+      emit_move_insn (temp, GEN_INT (sext_hwi (ud2 << 16, 32)));
       if (ud1 != 0)
-	emit_move_insn (copy_rtx (temp),
-			gen_rtx_IOR (DImode, copy_rtx (temp),
-				     GEN_INT (ud1)));
+	emit_move_insn (temp, gen_rtx_IOR (DImode, temp, GEN_INT (ud1)));
       emit_move_insn (dest, gen_rtx_AND (DImode, temp, GEN_INT (0xffffffff)));
     }
   else if (ud1 == ud3 && ud2 == ud4)
@@ -10282,18 +10275,13 @@  rs6000_emit_set_long_const (rtx dest, HOST_WIDE_INT c)
     {
       temp = !can_create_pseudo_p () ? dest : gen_reg_rtx (DImode);
 
-      emit_move_insn (copy_rtx (temp), GEN_INT (sext_hwi (ud3 << 16, 32)));
+      emit_move_insn (temp, GEN_INT (sext_hwi (ud3 << 16, 32)));
       if (ud2 != 0)
-	emit_move_insn (copy_rtx (temp),
-			gen_rtx_IOR (DImode, copy_rtx (temp),
-				     GEN_INT (ud2)));
-      emit_move_insn (ud1 != 0 ? copy_rtx (temp) : dest,
-		      gen_rtx_ASHIFT (DImode, copy_rtx (temp),
-				      GEN_INT (16)));
+	emit_move_insn (temp, gen_rtx_IOR (DImode, temp, GEN_INT (ud2)));
+      emit_move_insn (ud1 != 0 ? temp : dest,
+		      gen_rtx_ASHIFT (DImode, temp, GEN_INT (16)));
       if (ud1 != 0)
-	emit_move_insn (dest,
-			gen_rtx_IOR (DImode, copy_rtx (temp),
-				     GEN_INT (ud1)));
+	emit_move_insn (dest, gen_rtx_IOR (DImode, temp, GEN_INT (ud1)));
     }
   else if (TARGET_PREFIXED)
     {
@@ -10334,23 +10322,17 @@  rs6000_emit_set_long_const (rtx dest, HOST_WIDE_INT c)
     {
       temp = !can_create_pseudo_p () ? dest : gen_reg_rtx (DImode);
 
-      emit_move_insn (copy_rtx (temp), GEN_INT (sext_hwi (ud4 << 16, 32)));
+      emit_move_insn (temp, GEN_INT (sext_hwi (ud4 << 16, 32)));
       if (ud3 != 0)
-	emit_move_insn (copy_rtx (temp),
-			gen_rtx_IOR (DImode, copy_rtx (temp),
-				     GEN_INT (ud3)));
+	emit_move_insn (temp, gen_rtx_IOR (DImode, temp, GEN_INT (ud3)));
 
-      emit_move_insn (ud2 != 0 || ud1 != 0 ? copy_rtx (temp) : dest,
-		      gen_rtx_ASHIFT (DImode, copy_rtx (temp),
-				      GEN_INT (32)));
+      emit_move_insn (ud2 != 0 || ud1 != 0 ? temp : dest,
+		      gen_rtx_ASHIFT (DImode, temp, GEN_INT (32)));
       if (ud2 != 0)
-	emit_move_insn (ud1 != 0 ? copy_rtx (temp) : dest,
-			gen_rtx_IOR (DImode, copy_rtx (temp),
-				     GEN_INT (ud2 << 16)));
+	emit_move_insn (ud1 != 0 ? temp : dest,
+			gen_rtx_IOR (DImode, temp, GEN_INT (ud2 << 16)));
       if (ud1 != 0)
-	emit_move_insn (dest,
-			gen_rtx_IOR (DImode, copy_rtx (temp),
-				     GEN_INT (ud1)));
+	emit_move_insn (dest, gen_rtx_IOR (DImode, temp, GEN_INT (ud1)));
     }
 }