On Tue, Feb 16, 2021 at 12:39:39PM +0100, Uros Bizjak wrote:
> > @@ -5197,7 +5209,7 @@ (define_insn_and_split "*lea<mode>"
> >
> > /* ix86_avoid_lea_for_addr re-recognizes insn and may
> > change operands[] array behind our back. */
> > - pat = PATTERN (curr_insn);
> > + pat = PATTERN (peep2_next_insn (0));
>
> I remember there were several problems with the insn, so I introduced
> the above hack. Probably, there is better solution for this...
I'm afraid the define_peephole2 conditions are called with the same problem as
define_split conditions, operands point into recog_data.operands.
I wonder if instead of doing hacks like that we just couldn't save/restore
recog_data variable around the
dist_define = distance_non_agu_define (regno1, regno2, insn);
dist_use = distance_agu_use (regno0, insn);
calls.
As in the following updated patch:
2021-02-16 Jakub Jelinek <ja...@redhat.com>
PR target/99104
* config/i386/i386.c (distance_non_agu_define): Don't call
extract_insn_cached here.
(copy_recog_data): New function.
(ix86_lea_outperforms): Save and restore recog_data around call
to distance_non_agu_define and distance_agu_use.
(ix86_ok_to_clobber_flags): Remove.
(ix86_avoid_lea_for_add): Don't call ix86_ok_to_clobber_flags.
(ix86_avoid_lea_for_addr): Likewise. Adjust function comment.
* config/i386/i386.m (*lea<mode>): Change from define_insn_and_split
into define_insn. Move the splitting to define_peephole2 and
check there using peep2_regno_dead_p if FLAGS_REG is dead.
* gcc.dg/pr99104.c: New test.
--- gcc/config/i386/i386.c.jj 2021-02-16 08:58:55.197890195 +0100
+++ gcc/config/i386/i386.c 2021-02-16 13:47:15.500969539 +0100
@@ -14754,11 +14754,6 @@ distance_non_agu_define (unsigned int re
}
}
- /* get_attr_type may modify recog data. We want to make sure
- that recog data is valid for instruction INSN, on which
- distance_non_agu_define is called. INSN is unchanged here. */
- extract_insn_cached (insn);
-
if (!found)
return -1;
@@ -14897,6 +14892,32 @@ distance_agu_use (unsigned int regno0, r
return distance >> 1;
}
+/* Copy recog_data_d from SRC to DEST. */
+
+static void
+copy_recog_data (recog_data_d *dest, recog_data_d *src)
+{
+ dest->n_operands = src->n_operands;
+ dest->n_dups = src->n_dups;
+ dest->n_alternatives = src->n_alternatives;
+ dest->is_asm = src->is_asm;
+ dest->insn = src->insn;
+ for (int i = 0; i < src->n_operands; i++)
+ {
+ dest->operand[i] = src->operand[i];
+ dest->operand_loc[i] = src->operand_loc[i];
+ dest->constraints[i] = src->constraints[i];
+ dest->is_operator[i] = src->is_operator[i];
+ dest->operand_mode[i] = src->operand_mode[i];
+ dest->operand_type[i] = src->operand_type[i];
+ }
+ for (int i = 0; i < src->n_dups; i++)
+ {
+ dest->dup_loc[i] = src->dup_loc[i];
+ dest->dup_num[i] = src->dup_num[i];
+ }
+}
+
/* Define this macro to tune LEA priority vs ADD, it take effect when
there is a dilemma of choosing LEA or ADD
Negative value: ADD is more preferred than LEA
@@ -14928,17 +14949,16 @@ ix86_lea_outperforms (rtx_insn *insn, un
return true;
}
- rtx_insn *rinsn = recog_data.insn;
+ /* Remember recog_data content. */
+ struct recog_data_d recog_data_copy;
+ copy_recog_data (&recog_data_copy, &recog_data);
dist_define = distance_non_agu_define (regno1, regno2, insn);
dist_use = distance_agu_use (regno0, insn);
- /* distance_non_agu_define can call extract_insn_cached. If this function
- is called from define_split conditions, that can break insn splitting,
- because split_insns works by clearing recog_data.insn and then modifying
- recog_data.operand array and match the various split conditions. */
- if (recog_data.insn != rinsn)
- recog_data.insn = NULL;
+ /* distance_non_agu_define can call get_attr_type which can call
+ recog_memoized, restore recog_data back to previous content. */
+ copy_recog_data (&recog_data, &recog_data_copy);
if (dist_define < 0 || dist_define >= LEA_MAX_STALL)
{
@@ -14968,38 +14988,6 @@ ix86_lea_outperforms (rtx_insn *insn, un
return dist_define >= dist_use;
}
-/* Return true if it is legal to clobber flags by INSN and
- false otherwise. */
-
-static bool
-ix86_ok_to_clobber_flags (rtx_insn *insn)
-{
- basic_block bb = BLOCK_FOR_INSN (insn);
- df_ref use;
- bitmap live;
-
- while (insn)
- {
- if (NONDEBUG_INSN_P (insn))
- {
- FOR_EACH_INSN_USE (use, insn)
- if (DF_REF_REG_USE_P (use) && DF_REF_REGNO (use) == FLAGS_REG)
- return false;
-
- if (insn_defines_reg (FLAGS_REG, INVALID_REGNUM, insn))
- return true;
- }
-
- if (insn == BB_END (bb))
- break;
-
- insn = NEXT_INSN (insn);
- }
-
- live = df_get_live_out(bb);
- return !REGNO_REG_SET_P (live, FLAGS_REG);
-}
-
/* Return true if we need to split op0 = op1 + op2 into a sequence of
move and add to avoid AGU stalls. */
@@ -15012,10 +15000,6 @@ ix86_avoid_lea_for_add (rtx_insn *insn,
if (!TARGET_OPT_AGU || optimize_function_for_size_p (cfun))
return false;
- /* Check it is correct to split here. */
- if (!ix86_ok_to_clobber_flags(insn))
- return false;
-
regno0 = true_regnum (operands[0]);
regno1 = true_regnum (operands[1]);
regno2 = true_regnum (operands[2]);
@@ -15051,7 +15035,7 @@ ix86_use_lea_for_mov (rtx_insn *insn, rt
}
/* Return true if we need to split lea into a sequence of
- instructions to avoid AGU stalls. */
+ instructions to avoid AGU stalls during peephole2. */
bool
ix86_avoid_lea_for_addr (rtx_insn *insn, rtx operands[])
@@ -15071,10 +15055,6 @@ ix86_avoid_lea_for_addr (rtx_insn *insn,
&& REG_P (XEXP (operands[1], 0))))
return false;
- /* Check if it is OK to split here. */
- if (!ix86_ok_to_clobber_flags (insn))
- return false;
-
ok = ix86_decompose_address (operands[1], &parts);
gcc_assert (ok);
--- gcc/config/i386/i386.md.jj 2021-01-13 10:12:07.036506101 +0100
+++ gcc/config/i386/i386.md 2021-02-16 13:48:13.650317709 +0100
@@ -5176,7 +5176,7 @@ (define_expand "floatunsdidf2"
;; Load effective address instructions
-(define_insn_and_split "*lea<mode>"
+(define_insn "*lea<mode>"
[(set (match_operand:SWI48 0 "register_operand" "=r")
(match_operand:SWI48 1 "address_no_seg_operand" "Ts"))]
"ix86_hardreg_mov_ok (operands[0], operands[1])"
@@ -5189,38 +5189,36 @@ (define_insn_and_split "*lea<mode>"
else
return "lea{<imodesuffix>}\t{%E1, %0|%0, %E1}";
}
- "reload_completed && ix86_avoid_lea_for_addr (insn, operands)"
+ [(set_attr "type" "lea")
+ (set (attr "mode")
+ (if_then_else
+ (match_operand 1 "SImode_address_operand")
+ (const_string "SI")
+ (const_string "<MODE>")))])
+
+(define_peephole2
+ [(set (match_operand:SWI48 0 "register_operand")
+ (match_operand:SWI48 1 "address_no_seg_operand"))]
+ "ix86_hardreg_mov_ok (operands[0], operands[1])
+ && peep2_regno_dead_p (0, FLAGS_REG)
+ && ix86_avoid_lea_for_addr (peep2_next_insn (0), operands)"
[(const_int 0)]
{
machine_mode mode = <MODE>mode;
- rtx pat;
-
- /* ix86_avoid_lea_for_addr re-recognizes insn and may
- change operands[] array behind our back. */
- pat = PATTERN (curr_insn);
-
- operands[0] = SET_DEST (pat);
- operands[1] = SET_SRC (pat);
/* Emit all operations in SImode for zero-extended addresses. */
if (SImode_address_operand (operands[1], VOIDmode))
mode = SImode;
- ix86_split_lea_for_addr (curr_insn, operands, mode);
+ ix86_split_lea_for_addr (peep2_next_insn (0), operands, mode);
/* Zero-extend return register to DImode for zero-extended addresses. */
if (mode != <MODE>mode)
- emit_insn (gen_zero_extendsidi2
- (operands[0], gen_lowpart (mode, operands[0])));
+ emit_insn (gen_zero_extendsidi2 (operands[0],
+ gen_lowpart (mode, operands[0])));
DONE;
-}
- [(set_attr "type" "lea")
- (set (attr "mode")
- (if_then_else
- (match_operand 1 "SImode_address_operand")
- (const_string "SI")
- (const_string "<MODE>")))])
+})
;; Add instructions
--- gcc/testsuite/gcc.dg/pr99104.c.jj 2021-02-16 11:47:08.793255200 +0100
+++ gcc/testsuite/gcc.dg/pr99104.c 2021-02-16 11:47:08.793255200 +0100
@@ -0,0 +1,15 @@
+/* PR target/99104 */
+/* { dg-do compile { target int128 } } */
+/* { dg-options "-O2 -fsel-sched-pipelining -fselective-scheduling2
-funroll-loops" } */
+
+__int128 a;
+int b;
+int foo (void);
+
+int __attribute__ ((simd))
+bar (void)
+{
+ a = ~a;
+ if (foo ())
+ b = 0;
+}
Jakub
