Re: ssa bootstrap problem on x86 (cmpstrsi_1 pattern)

[Geoff Keating <geoffk@cygnus.com>]

> Cc: gcc@gcc.gnu.org
> X-URL: http://www.codesourcery.com
> Organization: CodeSourcery, LLC
> From: Mark Mitchell <mark@codesourcery.com>
> Date: Thu, 27 Jul 2000 13:10:11 -0700
> X-Dispatcher: imput version 990425(IM115)
> 
> >>>>> "Geoff" == Geoff Keating <geoffk@cygnus.com> writes:
> 
>     Geoff> ???  10035 is indeed valid in all such blocks.  It's not
>     Geoff> _useful_, since it holds an indeterminate value, but it has
>     Geoff> certainly been set, and it has only been set once.
>     Geoff> Hopefully, any kind of dataflow analysis we do would notice
>     Geoff> that it was set by a CLOBBER and therefore does not hold a
>     Geoff> useful value.
> 
> That would work -- but it misses one of the important benefits of SSA:
> you don't have to do dataflow analysis.  I think one of the big ideas
> is that if you're lower in the dominator tree you *know* what the
> values are for all registers in sight.  

I don't understand.  How do you know the value of a register which
is, say, a function parameter?

I also don't understand when you say "you don't have to do dataflow
analysis".  It is still not valid to turn

(set (reg 93) (const_int 7))
(set (reg 94) (plus (reg 93) (reg 95))

into 

(set (reg 94) (plus (reg 93) (reg 95))
(set (reg 93) (const_int 7))

nor is it valid to turn

(set (reg 93) (const_int 7))
(set (reg 94) (plus (reg 93) (reg 95))

into

(set (reg 93) (const_int 7))
(set (reg 94) (plus (reg 95) (const_int 42))

and to check both of these, you must do dataflow analysis.

SSA just makes this analysis much easier.  You don't have to worry
about where a register is set, you just find the one place where it is
set and you know that is the place.

Thus, if you see

(clobber (reg 93))

and somewhere else in the program, you see

(set (reg 94) (plus (reg 93) (reg 95))

you can simply turn the last insn into

(clobber (reg 94))

because you _know_ that reg 93 can't ever hold anything important; you
don't need to worry that there might be some other place where reg 93
gets a real value.  (I'm assuming that every register here is in SSA
form, of course.)


>     Geoff> Oops!  I must have sent the mail before I finished it.
>     Geoff> Yes, the idea would be to use
> 
>     Geoff> (clobber (match_scratch:SI "0"))
> 
> OK, I get it.  I think these two aspects are orthogonal: i386.md could
> be perhaps improved as you suggest -- and we still have to worry about
> what to do with CLOBBER in SSA.

How can they be orthogonal?  They are causing the bug together.

> I now totally understand where the motivation for your patch was
> coming from.  But, I still think that the fix isn't correct -- any
> more than the current code is correct.  I think we need to bite the
> bullet and get the CLOBBERs out of there; otherwise, we are forced to
> do some kind of dataflow anlaysis in SSA, and that negates one of the
> big wins from being in SSA form.  I think I can allocate some
> resources to solve this problem soon.

I am concerned you don't quite understand.  CLOBBERs can carry
information.  Deleting them deletes information.  Sometimes that
information can be deduced from the rest of the RTL, but sometimes it
cannot.

There is nothing inherently incompatible with CLOBBER and SSA.  The
problem you are seeing is with MATCH_DUP and SSA.  It just happens
that the particular pattern that causes the trouble uses CLOBBER, but
the problem would still have appeared if it used SET.  Try changing
the pattern so that instead of

(clobber (match_dup 0))
(clobber (match_dup 1))
(clobber (match_dup 2))

it says

(set (match_dup 0) (unspec [(mem:BLK (match_dup 0))
			    (mem:BLK (match_dup 1))
			    (match_dup 2)
			    (reg:CC 17)
			    (reg:SI 19)] 12))
(set (match_dup 1) (unspec [(mem:BLK (match_dup 0))
			    (mem:BLK (match_dup 1))
			    (match_dup 2)
			    (reg:CC 17)
			    (reg:SI 19)] 13))
(set (match_dup 2) (unspec [(mem:BLK (match_dup 0))
			    (mem:BLK (match_dup 1))
			    (match_dup 2)
			    (reg:CC 17)
			    (reg:SI 19)] 14))

which I believe is a fair statement of what this insn does, and you'll
see that nothing has changed---including that the bootstrap still
fails with -fssa.  You could easily "fix" this, by doing the
equivalent of your previous "fix" for SET instead of CLOBBER.  If you
can see why this would be wrong, then just switch over and you will
see why the CLOBBER fix is wrong.

> I feel clear on the concepts, now.  But, if you still disagree, let's
> keep on hashing it out, by all means.

I think the reason it's not clear why that line had to be there is
because the previous patch was only half of what I had planned to do.
I will write a bit more of the patch and perhaps you will see.

-- 
- Geoffrey Keating <geoffk@cygnus.com>