On Wed, May 13, 2020 at 5:58 PM H.J. Lu <hjl.to...@gmail.com> wrote:
> > > > The question is, why STV pass creates its funny sequence? The original
> > > > sequence should be easily solved by storing DImode from XMM register
> > > > and (with patched gcc) pushing DImode value from the same XMM
> > > > register.
> > >
> > > STV pass is mostly OK since it has to use XMM to move upper and lower
> > > 32 bits of a 64-bit integer. The problem is that push XMM becomes very
> > > expensive later.
> >
> > As shown in the patch, XMM push should be just decrement of SP reg and
> > move to the created stack slot.
>
> OK for master if there are no regressions?
diff --git a/gcc/config/i386/i386-features.c b/gcc/config/i386/i386-features.c
index 78fb373db6e..6cad125fa83 100644
--- a/gcc/config/i386/i386-features.c
+++ b/gcc/config/i386/i386-features.c
@@ -1264,7 +1264,8 @@ has_non_address_hard_reg (rtx_insn *insn)
FOR_EACH_INSN_DEF (ref, insn)
if (HARD_REGISTER_P (DF_REF_REAL_REG (ref))
&& !DF_REF_FLAGS_IS_SET (ref, DF_REF_MUST_CLOBBER)
- && DF_REF_REGNO (ref) != FLAGS_REG)
+ && DF_REF_REGNO (ref) != FLAGS_REG
+ && DF_REF_REGNO (ref) != SP_REG)
return true;
I don't think this part is correct. The function comment says:
/* Return 1 if INSN uses or defines a hard register.
Hard register uses in a memory address are ignored.
Clobbers and flags definitions are ignored. */
and you are putting SP_REG into clobber part.
OTOH, SP_REG in:
(insn 28 27 29 3 (set (mem:DI (pre_dec:SI (reg/f:SI 7 sp)) [2 S8 A64])
(reg/v:DI 85 [ target ])) "x.i":19:5 40 {*pushdi2}
is inside memory, so REG_SP should already be ignored. Please
investigate, why it is not the case.
+(define_insn "*pushv1ti2"
+ [(set (match_operand:V1TI 0 "push_operand" "=<")
+ (match_operand:V1TI 1 "general_no_elim_operand" "v"))]
+ ""
+ "#"
+ [(set_attr "type" "multi")
+ (set_attr "mode" "TI")])
+
+(define_split
+ [(set (match_operand:V1TI 0 "push_operand" "")
+ (match_operand:V1TI 1 "sse_reg_operand" ""))]
+ "TARGET_64BIT && reload_completed"
+ [(set (reg:P SP_REG) (plus:P (reg:P SP_REG) (match_dup 2)))
+ (set (match_dup 0) (match_dup 1))]
+{
+ operands[2] = GEN_INT (-PUSH_ROUNDING (16));
+ /* Preserve memory attributes. */
+ operands[0] = replace_equiv_address (operands[0], stack_pointer_rtx);
+})
The above part is wrong on many levels, e.g. using wrong predicate,
unnecessary rounding, it should be defined as define_insn_and_split,
conditionalized on TARGET_64BIT && TARGET_STV and put nearby existing
pushti patterns.
I will implement push changes (modulo V1T1mode) by myself, since they
are independent of STV stuff.
Uros.
