On Fri, Oct 20, 2023 at 4:33 PM Jeff Law <jeffreya...@gmail.com> wrote: > > > > On 10/20/23 03:53, Christoph Muellner wrote: > > From: Christoph Müllner <christoph.muell...@vrull.eu> > > > > This two patches add support for the XTheadMemIdx > > and XTheadFMemIdx ISA extensions, that support additional > > addressing modes. The extensions are implemented in a range > > of T-Head cores (e.g. C906, C910, C920) and are available > > on the market for quite some time. > > > > The ISA spec can be found here: > > https://github.com/T-head-Semi/thead-extension-spec > > > > An initial version of these patches has been sent a while ago. > > Jeff Law suggested to use INSNs instead of peepholes to let > > the combiner do the optimization. This is the major change > > that this patches have seen. > Did you happen to do any before/after testing? And if so, did using the > combiner help with discovery of these cases? I would expect it to have > done so, but it's always nice to have a confirmation.
I had no doubt this would be equal or better, therefore I did not plan to do that. However, measuring this is not that hard, so I just did the exercise of forward-porting the peephole-based patchset (and all tiny fixes that the v2 has). I then built xalancbmk_r/peak (randomly selected) with both compilers and compared the number of indexed loads and indexed stores in the binary: v1: 3982 indexed loads / 2447 indexed stores v2: 4110 indexed loads (+3.2%) / 2476 indexed stores (+1.2%) So your suggestion indeed helps to discover additional cases. Thanks again for that! BR Christoph
