>> 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? -- Regards Robin
