Hi!
On Mon, Aug 15, 2022 at 01:25:19PM +0800, Jiufu Guo wrote:
> This patch tries to put the constant into constant pool if building the
> constant requires 3 or more instructions.
>
> But there is a concern: I'm wondering if this patch is really profitable.
>
> Because, as I tested, 1. for simple case, if instructions are not been run
> in parallel, loading constant from memory maybe faster; but 2. if there
> are some instructions could run in parallel, loading constant from memory
> are not win comparing with building constant. As below examples.
>
> For f1.c and f3.c, 'loading' constant would be acceptable in runtime aspect;
> for f2.c and f4.c, 'loading' constant are visibly slower.
>
> For real-world cases, both kinds of code sequences exist.
>
> So, I'm not sure if we need to push this patch.
>
> Run a lot of times (1000000000) below functions to check runtime.
> f1.c:
> long foo (long *arg, long*, long *)
> {
> *arg = 0x1234567800000000;
> }
> asm building constant:
> lis 10,0x1234
> ori 10,10,0x5678
> sldi 10,10,32
> vs. asm loading
> addis 10,2,.LC0@toc@ha
> ld 10,.LC0@toc@l(10)
This is just a load insn, unless this is the only thing needing the TOC.
You can use crtl->uses_const_pool as an approximation here, to figure
out if we have that case?
> The runtime between 'building' and 'loading' are similar: some times the
> 'building' is faster; sometimes 'loading' is faster. And the difference is
> slight.
When there is only one constant, sure. But that isn't the expensive
case we need to avoid :-)
> addis 9,2,.LC2@toc@ha
> ld 7,.LC0@toc@l(7)
> ld 10,.LC1@toc@l(10)
> ld 9,.LC2@toc@l(9)
> For this case, 'loading' is always slower than 'building' (>15%).
Only if there is nothing else to do, and only in cases where code size
does not matter (i.e. microbenchmarks).
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr63281.c
> @@ -0,0 +1,11 @@
> +/* PR target/63281 */
> +/* { dg-do compile { target lp64 } } */
> +/* { dg-options "-O2 -std=c99" } */
Why std=c99 btw? The default is c17. Is there something we need to
disable here?
Segher
