xcore backend: add pattern for CP offset memory fetch

[Robert Lytton]

Hi Richard,
'specific' multiclass is unrolled.
test case moved into load.ll
thank you
robert

________________________________________
From: Richard Osborne
Sent: 02 July 2013 21:26
To: Robert Lytton
Cc: llvm-commits at cs.uiuc.edu
Subject: Re: xcore backend: add pattern for CP offset memory fetch

Hi Robert,

On 2 Jul 2013, at 17:53, Robert Lytton <robert at xmos.com> wrote:

>
> Hi,
>
> The XCore backend is fetching constants from the constant pool via an intermediate general register for the address.
> The 'CP' register plus immediate offset should be targeted instead - saving an instruction and temporary register.
>
> The patch below adds the missing "CP offset memory fetch" pattern to the target td.
>
> robert
>
>
> --- llvm/lib/Target/XCore/XCoreInstrInfo.td
> +++ llvm/lib/Target/XCore/XCoreInstrInfo.td
> @@ -283,7 +283,8 @@ multiclass FRU6_LRU6_cp<bits<6> opc, string OpcStr> {
>   def _ru6: _FRU6<opc, (outs RRegs:$a), (ins i32imm:$b),
>                   !strconcat(OpcStr, " $a, cp[$b]"), []>;
>   def _lru6: _FLRU6<opc, (outs RRegs:$a), (ins i32imm:$b),
> -                    !strconcat(OpcStr, " $a, cp[$b]"), []>;
> +                    !strconcat(OpcStr, " $a, cp[$b]"),
> +                    [(set RRegs:$a, (load (cprelwrapper tglobaladdr:$b)))]>;
> }

It seems wrong to hardcode a pattern for a specific instruction into a multiclass, when the benefit of multiclasses is that they can be instanced multiple times in order to factor out shared commonality that exists across multiple instructions. Since there is only one user of the FRU6_LRU6_cp multiclass would you be able to remove the multiclass and move everything to the LDWCP instructions instead?
>
> // U6
>
>
> --- llvm/test/CodeGen/XCore/constants.ll
> +++ llvm/test/CodeGen/XCore/constants.ll
> @@ -9,3 +9,16 @@ define i32 @f() {
> entry:
>        ret i32 12345678
> }
> +
> +; CHECK: g:
> +; CHECK: ldw [[REG:r[0-9]+]], cp[GConst]
> +; CHECK: stw [[REG]], dp[GData]
> +%struct = type <{ i32, i32 }>
> + at GConst = external constant %struct
> + at GData = external global %struct
> +define void @g() nounwind {
> +entry:
> +  %0 = load i32* getelementptr (%struct* @GConst, i32 0, i32 0)
> +  store i32 %0,  i32* getelementptr (%struct* @GData, i32 0, i32 0)
> +  ret void
> +}
>

Could you add the testcase to test/CodeGen/XCore/load.ll instead?

In future could you send patches as attachments instead of inline in the message (see http://llvm.org/docs/DeveloperPolicy.html#making-a-patch) - this makes it easier to apply patch.

Looks good to me otherwise. Would you be able to supply an updated patch?

Thanks,

Richard


-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch_xcore_load_cp
Type: application/octet-stream
Size: 1871 bytes
Desc: patch_xcore_load_cp
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130703/c62bf8dd/attachment.obj>