https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106594
--- Comment #16 from Segher Boessenkool <segher at gcc dot gnu.org> --- (In reply to Roger Sayle from comment #14) > This really is a regression, that points to a previously latent/unnoticed > bug in combine. > > In GCC 12, combine would take the input RTL and based on target costs > transform it into the better of implementation A or B. And it still does exactly that. And your patch would *not*! It does not compare the cost of two alternative pieces of code; it just says "if the cost of calculating some RTL expression as separate code is 4 or less, do not do any other optimisation here". It is completely ad-hoc, not an improvement, will only cause more problems in the future. > Now in GCC 13, the > tree-optimizers are able to perform this same optimization earlier and so > combine is now given optimal implementation A, So it is not a regression in combine. > where a latent bug always > transforms this to B without ever checking target costs. Latent. It is not a regression. > The consensus is that performing (more) optimizations at the tree-level is a > good thing, No. The situation is different, not so simplistic. At tree level the representation is higher level. At RTL level it is more nitty-gritty, very *low* level, very close to machine code. It is very important to do optimisations at that level as well. It is pretty much impossible to do a good job of low-level optimisations (like instruction selection) at a high level, i.e. at great distance. It is a bad idea to try to such things at a high level. What we can (and should, and *do*) do is to in higher level optimisations keep code flexible, only have abstractions that are easy to work with, etc.; and to have earlier passes output code that the later passes can work with well. > so reverting changes to the tree optimizers (that now produce > better code) is a workaround to a glitch where combine is transforming RTL > into more expensive forms. But no one is talking about reverting anything? > There's already a code path in combine that checks/compares costs, it just > isn't being reached any more. Now you completely lost me. > p.s. this has nothing to do with sign_extract/zero_extract, for which > hardware support would be hypothetical, this patch only affects > sign_extend/zero_extend, such as aarch64's sxtw or x86_64's movsx or > powerpc's extsw. If a target has this functionality, it's unlikely that a > sequence of shifts or bit-wise operations would be better. *_extract is just an example (see various open PRs) of the problems with compound_operation stuff. I would just rip out all that stuff, similar to your patch but a bit more drastic, except that causes regressions on other targets. Which I have tested and analysed, btw. As I said in the patch review, this really needs to be looked at on more targets. And it should be done in stage 1, not in stage 4. (I am running such a test with your patch on all Linux targets, fwiw).
