Bug 63390 - [SH] Hoist/schedule constant pool loads
Summary: [SH] Hoist/schedule constant pool loads
Status: UNCONFIRMED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 5.0
: P3 enhancement
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-09-27 18:53 UTC by Oleg Endo
Modified: 2014-09-28 22:26 UTC (History)
0 users

See Also:
Host:
Target: sh*-*-*
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Oleg Endo 2014-09-27 18:53:15 UTC
The following example resembles a use case where a tagged pointer (1 bit tag in LSB) is tested for nullptr and a function is invoked with the extracted normal pointer:

void foo (unsigned int, int);
void test (unsigned int* x)
{
  if (__builtin_expect ((*x >> 1) != 0, 1))
    foo (*x >> 1, 123456);
}

-O2 -m4 -ml:
        mov.l   @r4,r4
        shlr    r4
        tst     r4,r4
        bt      .L3
        mov.l   .L6,r1    <<<<
        mov.l   .L7,r5    <<<<
        jmp     @r1
        nop
        .align 1
.L3:
        rts	
        nop
.L7:

Since the pointer is likely to be not nullptr, the function 'foo' is also likely to be executed.  Thus it would make sense to hoist the loads for the function call to get better scheduling:

        mov.l   @r4,r4
        mov.l   .L6,r1    <<<<
        shlr    r4
        mov.l   .L7,r5    <<<<
        tst     r4,r4
        bt      .L3
        jmp     @r1
        nop
.L3:
        rts	
        nop
.L7:

The scheduler doesn't see that because the constant loads remain in the same basic block that contains the function call.  Moreover, the constant pool loads are expanded by sh_reorg pass after scheduling (which makes sense).

One idea could be to record the expanded constant pool load insns and try to hoist them after sh_reorg and maybe run the scheduler on the modified insn sections.
Comment 1 Oleg Endo 2014-09-28 18:41:18 UTC
Adding a loop to the example above shows that function address loads are hoisted already out of loops, but constant loads are not.

void foo (unsigned int, int);
void test (unsigned int* x, int c)
{
  while (--c)
  if (__builtin_expect ((*x >> 1) != 0, 1))
    foo (*x >> 1, 123456);
}

results in:
        mov.l   r8,@-r15
        mov     r5,r8
        mov.l   r9,@-r15
        mov     r4,r9
        mov.l   r10,@-r15
        mov.l   .L11,r10   <<<< function address load
        sts.l   pr,@-r15
.L2:
        dt      r8
        bt      .L10
        .align 2
.L4:
        mov.l   @r9,r4     <<<< constant load
        shlr    r4
        tst     r4,r4
        bt      .L2
        mov.l   .L12,r5
        jsr     @r10
        nop
        dt      r8
        bf      .L4
.L10:
        lds.l   @r15+,pr
        mov.l   @r15+,r10
        mov.l   @r15+,r9
        rts
        mov.l   @r15+,r8
Comment 2 Oleg Endo 2014-09-28 22:26:50 UTC
(In reply to Oleg Endo from comment #1)

> .L4:
>         mov.l   @r9,r4     <<<< constant load
>         shlr    r4
>         tst     r4,r4
>         bt      .L2
>         mov.l   .L12,r5
>         jsr     @r10
>         nop
>         dt      r8
>         bf      .L4
> .L10:
>         lds.l   @r15+,pr
>         mov.l   @r15+,r10
>         mov.l   @r15+,r9
>         rts
>         mov.l   @r15+,r8

I've marked the wrong load insn as constant load.  In this case hoisting it out of the loop wouldn't make lots of sense, since it's loaded into the call clobbered register r5.  Bad example.  But it would still be better to hoist the load into the preceding basic block.