This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: P3 SSE/MMX support: adding the patterns


On Wed, 6 Sep 2000, Richard Henderson wrote:

> I'd really prefer this be inside the switch statement.  This will
> force you to get the scheduling types correct.

OK.

> Need to add scheduling types for the non-split alternatives.

OK.

> There's no need for the first clause.  Constants are unique.

OK.

> > +(define_insn_and_split "*pushti"
> > +  [(set (match_operand:TI 0 "push_operand" "=<")
> > +	(match_operand:TI 1 "nonmemory_operand" "x"))]
> > +  "TARGET_SSE"
> > +  "#"
> > +  ""
> > +  [(set (reg:SI 7) (plus:SI (reg:SI 7) (const_int -16)))
> > +   (set (mem:TI (reg:SI 7)) (match_dup 1))]
> > +  "")
> 
> Perhaps we ought to make calls.c generate these?  Seems silly to
> have to define nine variants of the same thing.

It is silly, but the easiest thing to do.  I can think of two ways to
improve this: either implement a macro mechanism for md files, or fix
emit_push_insn so that it uses pushxx as a named pattern, and falls back
to add/move when it fails.  Both are likely to be quite a bit of extra
work (the latter because we may need to change a lot of ports).

> > +(define_insn "sse_movhps"
> > +  [(set (match_operand:V4SF 0 "nonimmediate_operand" "=x,m")
> > +	(vec_merge:V4SF
> > +	 (match_operand:V4SF 1 "nonimmediate_operand" "0,0")
> > +	 (match_operand:V4SF 2 "nonimmediate_operand" "m,x")
> > +	 (const_int 12)))]
> > +  "TARGET_SSE && (GET_CODE (operands[1]) == MEM || GET_CODE (operands[2]) == MEM)"
> > +  "movhps\\t{%2, %0|%0, %2}")
> 
> For normal named patterns it is illegal to refer to its operands
> in the extra_c_test.  Normally you'd have an expander that fixed
> things up to make sure you matched.  Is there something elsewhere
> that makes sure that things are correct?

This pattern is generated by the loadhps/storehps builtins, both of which
ensure that one argument is a MEM.

> > +(define_insn "sse_comi"
> > +  [(set (reg:CC 17)
> > +        (match_operator:CC 2 "sse_comparison_operator"
> > +
> > +(define_insn "sse_ucomi"
> > +  [(set (reg:CC 17)
> > +        (unspec:CC
> > +	 [(match_operator 2 "sse_comparison_operator"
> 
> Why not use CCFP and CCFPU modes to distinguish these instead
> of an unspec?

Hmm.. no reason I guess.  I'll change it.

> > +(define_insn "sfence"
> > +  [(unspec_volatile [(const_int 0)] 44)]
> > +  "TARGET_SSE"
> > +  "sfence")
> 
> You only need to prevent movement of memories across the insn.
> You might consider a definition like that for "mf" for ia64.

I have to admit I'm bewildered by the ia64 "mf" pattern.  It creates
  (mem:BLK (mem:BLK (scratch)))
and I fail to see the point of the two nested MEMs.

> > +(define_insn "prefetch"
> > +  [(unspec_volatile [(match_operand:SI 0 "address_operand" "p")
> > +		     (match_operand:SI 1 "address_operand" "p")] 35)]
> 
> Should just be unspec, not unspec_volatile.  No need to prevent
> scheduling across it.

Isn't the point where the prefetch is added rather critical for getting
good performance?  At least I think we should disallow scheduling memory
refernces across a prefetch, or we might end up moving the prefetch after
the "real" memory reference that it belongs to.

I'll submit a new version once we've sorted out the remaining problems.


Bernd


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]