diff mbox series

RISC-V: propgue/epilogue expansion code minor changes [NFC]

Message ID 20240515185505.143437-1-vineetg@rivosinc.com
State New
Headers show
Series RISC-V: propgue/epilogue expansion code minor changes [NFC] | expand

Commit Message

Vineet Gupta May 15, 2024, 6:55 p.m. UTC 
Saw this little room for improvement in current debugging of
prologue/epilogue expansion code.

---

Use the following pattern consistently
	`RTX_FRAME_RELATED_P (gen_insn (insn)) = 1`

vs. calling gen_insn around apriori gen_xxx_insn () calls.

This reduces weird indentations which are done inconsistently.

And also move the RTX_FRAME_RELATED_P () calls immediately after those
gen_xxx_insn () calls.

gcc/ChangeLog:
	* config/riscv/riscv.cc (riscv_expand_epilogue): Use pattern
	described above.
	(riscv_expand_prologue): Ditto.
	(riscv_for_each_saved_v_reg): Ditto.

Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
---
 gcc/config/riscv/riscv.cc | 54 ++++++++++++++++++---------------------
 1 file changed, 25 insertions(+), 29 deletions(-)

Comments

Jeff Law May 15, 2024, 7:32 p.m. UTC | #1 
On 5/15/24 12:55 PM, Vineet Gupta wrote:
> Saw this little room for improvement in current debugging of
> prologue/epilogue expansion code.
> 
> ---
> 
> Use the following pattern consistently
> 	`RTX_FRAME_RELATED_P (gen_insn (insn)) = 1`
> 
> vs. calling gen_insn around apriori gen_xxx_insn () calls.
> 
> This reduces weird indentations which are done inconsistently.
> 
> And also move the RTX_FRAME_RELATED_P () calls immediately after those
> gen_xxx_insn () calls.
> 
> gcc/ChangeLog:
> 	* config/riscv/riscv.cc (riscv_expand_epilogue): Use pattern
> 	described above.
> 	(riscv_expand_prologue): Ditto.
> 	(riscv_for_each_saved_v_reg): Ditto.
Thanks for cleaning this up.  Just having consistency is helpful.

All this gets scrambled again with stack-clash protection :(  But that's 
just the nature of the beast.

jeff
Vineet Gupta May 15, 2024, 8:51 p.m. UTC | #2 
On 5/15/24 12:32, Jeff Law wrote:
>
> On 5/15/24 12:55 PM, Vineet Gupta wrote:
>> Saw this little room for improvement in current debugging of
>> prologue/epilogue expansion code.
>>
>> ---
>>
>> Use the following pattern consistently
>> 	`RTX_FRAME_RELATED_P (gen_insn (insn)) = 1`
>>
>> vs. calling gen_insn around apriori gen_xxx_insn () calls.
>>
>> This reduces weird indentations which are done inconsistently.
>>
>> And also move the RTX_FRAME_RELATED_P () calls immediately after those
>> gen_xxx_insn () calls.
>>
>> gcc/ChangeLog:
>> 	* config/riscv/riscv.cc (riscv_expand_epilogue): Use pattern
>> 	described above.
>> 	(riscv_expand_prologue): Ditto.
>> 	(riscv_for_each_saved_v_reg): Ditto.
> Thanks for cleaning this up.  Just having consistency is helpful.
>
> All this gets scrambled again with stack-clash protection :(  But that's 
> just the nature of the beast.

Apparently it couldn't clear CI - so much for an NFC.
And I can now see why:

-      insn = emit_insn (riscv_gen_gpr_save_insn (frame));
+      insn = riscv_gen_gpr_save_insn (frame);
+      RTX_FRAME_RELATED_P (emit_insn (insn)) = 1;
...
 
-      RTX_FRAME_RELATED_P (insn) = 1;
       REG_NOTES (insn) = dwarf;

The REG_NOTE is being added to older rtx insn lacking the
RTX_FRAME_RELATED_P tagging.

-Vineet
diff mbox series

Patch

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 4067505270e1..6d95e2d41e87 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -7456,15 +7456,14 @@  riscv_for_each_saved_v_reg (poly_int64 &remaining_size,
 		if (CONST_INT_P (vlen))
 		  {
 		    gcc_assert (SMALL_OPERAND (-INTVAL (vlen)));
-		    insn = emit_insn (gen_add3_insn (stack_pointer_rtx,
-						     stack_pointer_rtx,
-						     GEN_INT (-INTVAL (vlen))));
+		    insn = gen_add3_insn (stack_pointer_rtx, stack_pointer_rtx,
+					  GEN_INT (-INTVAL (vlen)));
 		  }
 		else
-		  insn = emit_insn (
-		    gen_sub3_insn (stack_pointer_rtx, stack_pointer_rtx, vlen));
+		  insn = gen_sub3_insn (stack_pointer_rtx, stack_pointer_rtx,
+					vlen);
 		gcc_assert (insn != NULL_RTX);
-		RTX_FRAME_RELATED_P (insn) = 1;
+		RTX_FRAME_RELATED_P (emit_insn (insn)) = 1;
 		riscv_save_restore_reg (m1_mode, regno, 0, fn);
 		remaining_size -= UNITS_PER_V_REG;
 	      }
@@ -7481,10 +7480,10 @@  riscv_for_each_saved_v_reg (poly_int64 &remaining_size,
 	    if (handle_reg)
 	      {
 		riscv_save_restore_reg (m1_mode, regno, 0, fn);
-		rtx insn = emit_insn (
-		  gen_add3_insn (stack_pointer_rtx, stack_pointer_rtx, vlen));
+		rtx insn = gen_add3_insn (stack_pointer_rtx, stack_pointer_rtx,
+					  vlen);
 		gcc_assert (insn != NULL_RTX);
-		RTX_FRAME_RELATED_P (insn) = 1;
+		RTX_FRAME_RELATED_P (emit_insn (insn)) = 1;
 		remaining_size -= UNITS_PER_V_REG;
 	      }
 	  }
@@ -7730,10 +7729,10 @@  riscv_expand_prologue (void)
 
       /* emit multi push insn & dwarf along with it.  */
       stack_adj = frame->multi_push_adj_base + multi_push_additional;
-      insn = emit_insn (riscv_gen_multi_push_pop_insn (
-	PUSH_IDX, -stack_adj, riscv_multi_push_regs_count (frame->mask)));
+      insn = riscv_gen_multi_push_pop_insn (
+	PUSH_IDX, -stack_adj, riscv_multi_push_regs_count (frame->mask));
+      RTX_FRAME_RELATED_P (emit_insn (insn)) = 1;
       dwarf = riscv_adjust_multi_push_cfi_prologue (stack_adj);
-      RTX_FRAME_RELATED_P (insn) = 1;
       REG_NOTES (insn) = dwarf;
 
       /* Temporarily fib that we need not save GPRs.  */
@@ -7757,10 +7756,10 @@  riscv_expand_prologue (void)
       dwarf = riscv_adjust_libcall_cfi_prologue ();
 
       remaining_size -= frame->save_libcall_adjustment;
-      insn = emit_insn (riscv_gen_gpr_save_insn (frame));
+      insn = riscv_gen_gpr_save_insn (frame);
+      RTX_FRAME_RELATED_P (emit_insn (insn)) = 1;
       frame->mask = 0; /* Temporarily fib that we need not save GPRs.  */
 
-      RTX_FRAME_RELATED_P (insn) = 1;
       REG_NOTES (insn) = dwarf;
     }
 
@@ -7779,10 +7778,10 @@  riscv_expand_prologue (void)
       frame->gp_sp_offset -= save_adjustment;
       remaining_size -= save_adjustment;
 
-      insn = emit_insn (gen_th_int_push ());
+      insn = gen_th_int_push ();
+      RTX_FRAME_RELATED_P (emit_insn (insn)) = 1;
 
       rtx dwarf = th_int_adjust_cfi_prologue (th_int_mask);
-      RTX_FRAME_RELATED_P (insn) = 1;
       REG_NOTES (insn) = dwarf;
     }
 
@@ -8084,9 +8083,8 @@  riscv_expand_epilogue (int style)
 	    adjust = GEN_INT (adjust_offset.to_constant ());
 	}
 
-      insn = emit_insn (
-	       gen_add3_insn (stack_pointer_rtx, hard_frame_pointer_rtx,
-			      adjust));
+      insn = gen_add3_insn (stack_pointer_rtx, hard_frame_pointer_rtx, adjust);
+      RTX_FRAME_RELATED_P (emit_insn (insn)) = 1;
 
       rtx dwarf = NULL_RTX;
       rtx cfa_adjust_value = gen_rtx_PLUS (
@@ -8094,7 +8092,6 @@  riscv_expand_epilogue (int style)
 			       gen_int_mode (-frame->hard_frame_pointer_offset, Pmode));
       rtx cfa_adjust_rtx = gen_rtx_SET (stack_pointer_rtx, cfa_adjust_value);
       dwarf = alloc_reg_note (REG_CFA_ADJUST_CFA, cfa_adjust_rtx, dwarf);
-      RTX_FRAME_RELATED_P (insn) = 1;
 
       REG_NOTES (insn) = dwarf;
     }
@@ -8157,9 +8154,9 @@  riscv_expand_epilogue (int style)
 	      adjust = RISCV_PROLOGUE_TEMP (Pmode);
 	    }
 
-	  insn = emit_insn (gen_add3_insn (stack_pointer_rtx,
-					   stack_pointer_rtx,
-					   adjust));
+	  insn = gen_add3_insn (stack_pointer_rtx, stack_pointer_rtx, adjust);
+	  RTX_FRAME_RELATED_P (emit_insn (insn)) = 1;
+
 	  rtx dwarf = NULL_RTX;
 	  rtx cfa_adjust_rtx
 	    = gen_rtx_PLUS (Pmode, stack_pointer_rtx,
@@ -8167,7 +8164,6 @@  riscv_expand_epilogue (int style)
 					  Pmode));
 
 	  dwarf = alloc_reg_note (REG_CFA_DEF_CFA, cfa_adjust_rtx, dwarf);
-	  RTX_FRAME_RELATED_P (insn) = 1;
 
 	  REG_NOTES (insn) = dwarf;
 	}
@@ -8227,15 +8223,15 @@  riscv_expand_epilogue (int style)
   /* Deallocate the final bit of the frame.  */
   if (step2.to_constant () > 0)
     {
-      insn = emit_insn (gen_add3_insn (stack_pointer_rtx, stack_pointer_rtx,
-				       GEN_INT (step2.to_constant ())));
+      insn = gen_add3_insn (stack_pointer_rtx, stack_pointer_rtx,
+			    GEN_INT (step2.to_constant ()));
+      RTX_FRAME_RELATED_P (emit_insn (insn)) = 1;
 
       rtx dwarf = NULL_RTX;
       rtx cfa_adjust_rtx
 	= gen_rtx_PLUS (Pmode, stack_pointer_rtx,
 			GEN_INT (libcall_size + multipop_size));
       dwarf = alloc_reg_note (REG_CFA_DEF_CFA, cfa_adjust_rtx, dwarf);
-      RTX_FRAME_RELATED_P (insn) = 1;
 
       REG_NOTES (insn) = dwarf;
     }
@@ -8260,8 +8256,8 @@  riscv_expand_epilogue (int style)
   else if (use_restore_libcall)
     {
       rtx dwarf = riscv_adjust_libcall_cfi_epilogue ();
-      insn = emit_insn (gen_gpr_restore (GEN_INT (riscv_save_libcall_count (mask))));
-      RTX_FRAME_RELATED_P (insn) = 1;
+      insn = gen_gpr_restore (GEN_INT (riscv_save_libcall_count (mask)));
+      RTX_FRAME_RELATED_P (emit_insn (insn)) = 1;
       REG_NOTES (insn) = dwarf;
 
       emit_jump_insn (gen_gpr_restore_return (ra));