"Robin Dapp" <rdapp....@gmail.com> writes: >>> If the problem is tracking liveness, wouldn't it be better to >>> iterate over the "then" block in reverse order? We would start >>> with the liveness set for the join block and update as we move >>> backwards through the "then" block. This liveness set would >>> tell us whether the current instruction needs to preserve a >>> particular register. That should make it possible to do the >>> transformation in one step, and so avoid the risk that the >>> second attempt does something that is unexpectedly different >>> from the first attempt. >> >> I agree that the current approach is rather cumbersome. Indeed >> the second attempt was conditional at first and I changed it to >> be unconditional after some patch iterations. >> Your reverse-order idea sounds like it should work. To further >> clean up the algorithm we could also make it more explicit >> that a "cmov" depends on either the condition or the CC and >> basically track two separate paths through the block, one CC >> path and one "condition" path. > > I gave this another thought. Right now we keep track of the > generated targets and temporaries in forward order, using those > for the source rewiring. I don't see how we could do that in > reverse order other than have another fixup iteration afterwards. > >>> FWIW, the reason for asking was that it seemed safer to pass >>> use_cond_earliest back from noce_convert_multiple_sets_1 >>> to noce_convert_multiple_sets, as another parameter, >>> and then do the adjustment around noce_convert_multiple_sets's >>> call to targetm.noce_conversion_profitable_p. That would avoid >>> the new for a new if_info field, which in turn would make it >>> less likely that stale information is carried over from one attempt >>> to the next (e.g. if other ifcvt techniques end up using the same >>> field in future). >> >> Would something like the attached v4 be OK that uses a parameter >> instead (I mean without having refactored the full algorithm)? >> At least I changed the comment before the second attempt to >> hopefully cause a tiny bit less confusion :) >> I haven't fully bootstrapped it yet. > > That v4 was bootstrapped and regtested on x86 and aarch64 in the meanwhile and > it has been in our internal tree for a while without problems. > > Would it be OK for trunk without further refactoring?
I think it'd be better if I abstain from this. I probably disagree too much with the current structure and the way that the code is developing. I won't object if anyone else approves it though. Thanks, Richard
