Hi!
On 2021-03-17T08:54:15+0100, Richard Biener <richard.guent...@gmail.com> wrote:
> On Wed, Mar 17, 2021 at 12:35 AM Thomas Schwinge
> <tho...@codesourcery.com> wrote:
>> On 2021-03-17T00:24:55+0100, I wrote:
>> > Now, walking each function backwards (!), [...]
>>
>> > I've now got a simple 'callback_op', which for '!is_lhs' looks at
>> > 'get_base_address ([op])', and if that 'var' is contained in the set of
>> > current candidates (initialized per containg 'bind's, which we enter
>> > first, even if walking a 'gimple_seq' backwards), removes that 'var' as a
>> > candidate for such optimization. (Plus some "details", of couse.) This
>> > seems to work fine, as far as I can tell. :-)
>>
>> Is something like the attached "[WIP] 'walk_gimple_seq' backward" OK
>> conceptually? (For next development stage 1 and with all the TODOs
>> resolved, of course.)
>>
>> The 'backward' flag cannot simply be a argument to 'walk_gimple_seq'
>> etc.: it needs to also be passed to 'walk_gimple_seq_mod' calls triggered
>> from inside 'walk_gimple_stmt'. Hence, I've put it into the state
>> 'struct walk_stmt_info'.
>
> Can't you simply walk the sequence backward youself and call
> walk_gimple_stmt on each stmt instead?
That's what I'd intended to convey with the paragraph above -- I can't do
what you suggested: 'walk_gimple_stmt' will recursively call
'walk_gimple_seq_mod' ("If STMT can have statements inside
(e.g. GIMPLE_BIND), walk them.") -- and I need all these to be walked
backwards, too. (..., and don't want to re-produce that whole
'walk_gimple_stmt' logic -- but could factor that logic out of
'walk_gimple_stmt', if that's cleaner than the 'backward' flag?)
> That said,
>
> if (!wi->removed_stmt)
> - gsi_next (&gsi);
> + {
> + if (forward)
> + gsi_next (&gsi);
> + else //TODO Correct?
> + gsi_prev (&gsi);
> + //TODO This could do with some unit testing, to make sure
> all the corner cases (removing first/last, for example) work
> correctly.
> + }
>
> if wi->removed_stmt maps to gsi_remove being called then the backwards
> code is incorrect since gsi_remove advances the iterator in the wrong
> direction.
> So you always need gsi_prev () here.
Thanks, we shall look into that.
> Otherwise sure.
OK, we shall prepare something for next development stage 1.
Grüße
Thomas
-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank
Thürauf
