On Wed, Oct 23, 2024 at 04:27:29PM +0200, Konstantinos Eleftheriou wrote:
Just random ChangeLog formatting nits, not actual patch review:
> gcc/ChangeLog:
>
> * Makefile.in: Add avoid-store-forwarding.o
Missing . at the end. Though, you should really also mention
what you're changing, so
* Makefile.in (OBJS): Add avoid-store-forwarding.o.
> * common.opt: New option -favoid-store-forwarding.
* common.opt (favoid-store-forwarding): New option.
> * common.opt.urls: Regenerate.
> * doc/invoke.texi: New param store-forwarding-max-distance.
> * doc/passes.texi: Document new pass.
> * doc/tm.texi: Regenerate.
> * doc/tm.texi.in: Document new pass.
> * params.opt: New param store-forwarding-max-distance.
* params.opt (store-forwarding-max-distance): New param.
> * passes.def: Schedule a new pass.
This doesn't say what you schedule and where.
* passes.def: Add pass_rtl_avoid_store_forwarding before
pass_early_remat.
or so.
> * target.def (HOOK_PREFIX): New target hook avoid_store_forwarding_p.
Better
* target.def (avoid_store_forwarding_p): New DEFHOOK.
> * target.h (struct store_fwd_info): Declare.
> * targhooks.cc (default_avoid_store_forwarding_p):
> Add default_avoid_store_forwarding_p.
We don't indent the subsequent lines further than by a single tab. Plus
it is just a new function, so just say
* targhooks.cc (default_avoid_store_forwarding_p): New function.
> * targhooks.h (default_avoid_store_forwarding_p):
> Add default_avoid_store_forwarding_p.
See above, but just: Declare.
> * tree-pass.h (make_pass_rtl_avoid_store_forwarding): Declare.
> * avoid-store-forwarding.cc: New file.
> * avoid-store-forwarding.h: New file.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/aarch64/avoid-store-forwarding-1.c: New test.
> * gcc.target/aarch64/avoid-store-forwarding-2.c: New test.
> * gcc.target/aarch64/avoid-store-forwarding-3.c: New test.
> * gcc.target/aarch64/avoid-store-forwarding-4.c: New test (XFAIL).
> * gcc.target/aarch64/avoid-store-forwarding-5.c: New test (XFAIL).
The " (XFAIL)" parts IMHO don't belong here.
And, it would be nice to get some test coverage on other arches too.
Jakub
