diff mbox series

[avr] Fix PR116953 trouble with global state recog_data

Message ID 21046182-4208-40fa-bf1b-7ac51d944df2@gjlay.de
State New
Headers show
Series [avr] Fix PR116953 trouble with global state recog_data | expand

Commit Message

Georg-Johann Lay Oct. 22, 2024, 10:19 a.m. UTC 
This patch is a 2nd take on fixing PR116953:

The output function for avr_out_sbxx_branch() runs
jump_over_one_insn_p() which calls extract for the next insn, which
clobbers recog_data.  The previous fix makes local copy of the input
operands[] to avr_out_sbxx_branch() -- which is recog_data.operand.
However, avr_out_sbxx_branch() may also return asm output template
code with %-operands, so that recog_data is referred to elsewhere,
like in print_operand.

The attached fix reverts that and just extracts the current insn's
operands and stuff again.

Unfortunately I don't have a test case for this; I came across
it when working on some feature patch.

Ok for trunk and releases/gcc-14 ? (Which have the previous fix).

Johann

--

AVR: target/116953 - Restore recog_data after calling jump_over_one_insn_p.

The previous fix for PR116953 is incomplete because references to
recog_data are escaping avr_out_sbxx_branch() in the form of %-operands
in the returned asm code template.  This patch reverts the previous fix,
and re-extracts the operands by means of extract_constrain_insn_cached()
after the call of jump_over_one_insn_p().

	PR target/116953
gcc/
	* config/avr/avr.cc (avr_out_sbxx_branch): Revert previous fix
	for PR116953 (r15-4078).  Run extract_constrain_insn_cached
	on the current insn after calling jump_over_one_insn_p.

Comments

Denis Chertykov Oct. 22, 2024, 3:01 p.m. UTC | #1 
вт, 22 окт. 2024 г. в 14:19, Georg-Johann Lay <avr@gjlay.de>:
>
> This patch is a 2nd take on fixing PR116953:
>
> The output function for avr_out_sbxx_branch() runs
> jump_over_one_insn_p() which calls extract for the next insn, which
> clobbers recog_data.  The previous fix makes local copy of the input
> operands[] to avr_out_sbxx_branch() -- which is recog_data.operand.
> However, avr_out_sbxx_branch() may also return asm output template
> code with %-operands, so that recog_data is referred to elsewhere,
> like in print_operand.
>
> The attached fix reverts that and just extracts the current insn's
> operands and stuff again.
>
> Unfortunately I don't have a test case for this; I came across
> it when working on some feature patch.
>
> Ok for trunk and releases/gcc-14 ? (Which have the previous fix).

Ok.
Please apply.

Denis.

>
> Johann
>
> --
>
> AVR: target/116953 - Restore recog_data after calling jump_over_one_insn_p.
>
> The previous fix for PR116953 is incomplete because references to
> recog_data are escaping avr_out_sbxx_branch() in the form of %-operands
> in the returned asm code template.  This patch reverts the previous fix,
> and re-extracts the operands by means of extract_constrain_insn_cached()
> after the call of jump_over_one_insn_p().
>
>         PR target/116953
> gcc/
>         * config/avr/avr.cc (avr_out_sbxx_branch): Revert previous fix
>         for PR116953 (r15-4078).  Run extract_constrain_insn_cached
>         on the current insn after calling jump_over_one_insn_p.
diff mbox series

Patch

    AVR: target/116953 - Restore recog_data after calling jump_over_one_insn_p.
    
    The previous fix for PR116953 is incomplete because references to
    recog_data are escaping avr_out_sbxx_branch() in the form of %-operands
    in the returned asm code template.  This patch reverts the previous fix,
    and re-extracts the operands by means of extract_constrain_insn_cached()
    after the call of jump_over_one_insn_p().
    
            PR target/116953
    gcc/
            * config/avr/avr.cc (avr_out_sbxx_branch): Revert previous fix
            for PR116953 (r15-4078).  Run extract_constrain_insn_cached
            on the current insn after calling jump_over_one_insn_p.

diff --git a/gcc/config/avr/avr.cc b/gcc/config/avr/avr.cc
index 7d0d2b16c18..55691de4177 100644
--- a/gcc/config/avr/avr.cc
+++ b/gcc/config/avr/avr.cc
@@ -13597,16 +13597,16 @@  avr_hard_regno_rename_ok (unsigned int old_reg, unsigned int new_reg)
    Operand 3: label to jump to if the test is true.  */
 
 const char *
-avr_out_sbxx_branch (rtx_insn *insn, rtx xop[])
+avr_out_sbxx_branch (rtx_insn *insn, rtx operands[])
 {
-  // jump_over_one_insn_p may call extract on the next insn, clobbering
-  // recog_data.operand.  Hence make a copy of the operands (PR116953).
-  rtx operands[] = { xop[0], xop[1], xop[2], xop[3] };
-
   rtx_code comp = GET_CODE (operands[0]);
   bool long_jump = get_attr_length (insn) >= 4;
   bool reverse = long_jump || jump_over_one_insn_p (insn, operands[3]);
 
+  // PR116953: jump_over_one_insn_p may call extract on the next insn,
+  // clobbering recog_data.operand.  Thus, restore recog_data.
+  extract_constrain_insn_cached (insn);
+
   if (comp == GE)
     comp = EQ;
   else if (comp == LT)