Re: [PATCH]: PRE can handle stuff set in jumps

[Daniel Berlin <dan at cgsoftware dot com>]

Daniel Berlin <dan@cgsoftware.com> writes:

> law@redhat.com writes:
>
>>   In message <877kvy9zly.fsf@cgsoftware.com>you write:
>>   > As I mentioned to Geoff, PRE inserts on edges, and thus, can handle
>>   > sets in jumps. If it can't, then something in the edge insertion is
>>   > broken. 
>>   > Thus, saying an expression inside a jump isn't available is incorrect
>>   > unless we are running classic GCSE.
>>   > It appears the patch was committed anyway a few days ago (I never
>>   > noticed), so here's a simple fix,  since classic GCSE is only run if
>>   > we are optimizing for size. 
>>   > 
>>   >    2001-08-20  Daniel Berlin  <dan@cgsoftware.com>
>>   > 
>>   > 	* gcse.c (hash_scan_set): It's only not available in jumps if this
>>   > 	is classic GCSE. Update comment and code to reflect that.
>> Actually, you're missing the whole point behind Geoff's patch (which Geoff
>> and I discussed at length before it was posted).
> Errr, no i'm not, unless he's pointed out the wrong part of the code
> as being incorrect.
> He pointed to part of handle_avail_expr as being the code in error.
> handle_avail_expr is only called from classic GCSE.
> Therefore, changing hash_scan_set is the wrong solution, unless you
> restrict the change to classic gcse's calling of hash_scan_set
> somehow.
Let's run through this again, in case you don't get it.

He says handle_avail_expr is inserting in the wrong place.
He changes hash_scan_set to not mark the expression as available in
the first place.
However, handle_avail_expr is only used for classic GCSE.
While hash_scan_set is called by PRE as well (and const/copy prop, but it's
a different if block than the one he changed).

So by making the change there, he's disabled what should be working
just fine for PRE.  This is all under the assumption the part he says
is broken, is the part that is broken.
Also note that people outside of redhat have no testcase where it fails at
all, for either classic  GCSE or PRE. So nobody except redhat people
can even say *what* part is broken, and thus, i have to rely on the
info given.
And given what Geoff said, his patch is wrong.
It disables expressions in jumps from being available for both classic
GCSE, and PRE, when the problem pointed out can only occur for classic
GCSE.
So please don't tell me i'm missing the whole point of his patch.
There was no followup by him when i pointed out the code he said was
wrong only occurs for classic GCSE.
The patch was just committed about a week later.

>
>>
>> While it is possible to make gcse do the right thing in Geoff's case, I
>> do not believe it is worth the effort.  Far from it.
>
>>
>> jeff
>
> -- 
> "I went to the bank and asked to borrow a cup of money.  They
> said, "What for?"  I said, "I'm going to buy some sugar."
> "-Steven Wright

-- 
"Friday, I was in a bookstore and I started talking to a French
looking girl.  She was a bilingual illiterate -- she couldn't
read in two different languages.
"-Steven Wright