[PATCH] R600/SI: Move SIFixSGPRCopies to right after isel

Tom Stellard tom at stellard.net
Mon Nov 17 12:07:38 PST 2014 

On Fri, Nov 14, 2014 at 11:01:07AM -0800, Matt Arsenault wrote:
> 
> > On Oct 16, 2014, at 5:58 PM, Matt Arsenault <arsenm2 at gmail.com> wrote:
> > 
> > Hi,
> > 
> > These move running SIFixSGPRCopies to be a part of instruction selection. This depends on these other patches to eliminate the use of the rsrc pseudos to avoid needing to make moveToVALU handle them: http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20141013/240045.html
> > 
> > <0001-R600-SI-Assume-SIFixSGPRCopies-makes-changes.patch><0002-R600-SI-Don-t-copy-flags-when-extracting-subreg.patch><0003-R600-SI-Move-SIFixSGPRCopies-to-inst-selector-passes.patch>
> 
> 
> Updated patches
> 

> From 21a4f3cd57065977c920bed31708fa373465acdf Mon Sep 17 00:00:00 2001
> From: Matt Arsenault <Matthew.Arsenault at amd.com>
> Date: Mon, 13 Oct 2014 23:30:19 -0700
> Subject: [PATCH 1/3] R600/SI: Assume SIFixSGPRCopies makes changes
> 
> I'm not sure if this was breaking anything.
> ---
>  lib/Target/R600/SIFixSGPRCopies.cpp | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 

LGTM.

> From dc6d5f9566b948738c0d5d62018907b11292b408 Mon Sep 17 00:00:00 2001
> From: Matt Arsenault <Matthew.Arsenault at amd.com>
> Date: Mon, 13 Oct 2014 23:29:16 -0700
> Subject: [PATCH 2/3] R600/SI: Don't copy flags when extracting subreg
> 
> This was resulting in use of a register after a kill.
> For some reason this showed up as a problem in many tests
> when moving the SIFixSGPRCopies pass closer to instruction
> selection.
> ---
>  lib/Target/R600/SIInstrInfo.cpp | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 

LGTM.

> From 295f0d89b0af55283a7feaee39333b584ad2d399 Mon Sep 17 00:00:00 2001
> From: Matt Arsenault <Matthew.Arsenault at amd.com>
> Date: Mon, 6 Oct 2014 20:56:54 -0700
> Subject: [PATCH 3/3] R600/SI: Move SIFixSGPRCopies to inst selector passes
> 
> This should expose more of the actually used VALU
> instructions to the machine optimization passes.
> 
> This also should help getting i1 handling into a better state.
> For not entirly understood reasons, this fixes the split-scalar-i64-add.ll
> test where a 64-bit add would only partially be moved to the VALU
> resulting in use of undefined VCC.
> ---
>  lib/Target/R600/AMDGPUTargetMachine.cpp   | 21 ++++++++++++++-------
>  test/CodeGen/R600/i1-copy-implicit-def.ll |  2 +-
>  test/CodeGen/R600/i1-copy-phi.ll          | 12 ++++++++++--
>  test/CodeGen/R600/split-scalar-i64-add.ll |  7 ++++++-
>  4 files changed, 31 insertions(+), 11 deletions(-)

To avoid a regression, the attached patch should be committed before this one.

> 
> diff --git a/lib/Target/R600/AMDGPUTargetMachine.cpp b/lib/Target/R600/AMDGPUTargetMachine.cpp
> index 6f8ce1a..04a4f87 100644
> --- a/lib/Target/R600/AMDGPUTargetMachine.cpp
> +++ b/lib/Target/R600/AMDGPUTargetMachine.cpp
> @@ -150,8 +150,20 @@ AMDGPUPassConfig::addPreISel() {
>  }
>  
>  bool AMDGPUPassConfig::addInstSelector() {
> +  const AMDGPUSubtarget &ST = TM->getSubtarget<AMDGPUSubtarget>();
> +
>    addPass(createAMDGPUISelDag(getAMDGPUTargetMachine()));
> -  addPass(createSILowerI1CopiesPass());
> +
> +  if (ST.getGeneration() >= AMDGPUSubtarget::SOUTHERN_ISLANDS) {
> +    addPass(createSILowerI1CopiesPass());
> +
> +    addPass(createSIFixSGPRCopiesPass(*TM));
> +    // SIFixSGPRCopies can generate a lot of duplicate instructions,
> +    // so we need to run MachineCSE afterwards.
> +    // XXX - Should we remove this since it will be running anyway from here
> +    addPass(&MachineCSEID);

We don't need this anymore.

> +  }
> +
>    return false;
>  }
>  
> @@ -161,12 +173,7 @@ bool AMDGPUPassConfig::addPreRegAlloc() {
>    if (ST.getGeneration() <= AMDGPUSubtarget::NORTHERN_ISLANDS) {
>      addPass(createR600VectorRegMerger(*TM));
>    } else {
> -    addPass(createSIFixSGPRCopiesPass(*TM));
> -    // SIFixSGPRCopies can generate a lot of duplicate instructions,
> -    // so we need to run MachineCSE afterwards.
> -    addPass(&MachineCSEID);
> -
> -    if (getOptLevel() > CodeGenOpt::None && ST.loadStoreOptEnabled()) {
> +     if (getOptLevel() > CodeGenOpt::None && ST.loadStoreOptEnabled()) {

Random whitespace change.

LGTM.

>        // Don't do this with no optimizations since it throws away debug info by
>        // merging nonadjacent loads.
>  
> diff --git a/test/CodeGen/R600/i1-copy-implicit-def.ll b/test/CodeGen/R600/i1-copy-implicit-def.ll
> index 4af3879..7c5bc04 100644
> --- a/test/CodeGen/R600/i1-copy-implicit-def.ll
> +++ b/test/CodeGen/R600/i1-copy-implicit-def.ll
> @@ -1,7 +1,7 @@
>  ; RUN: llc -march=r600 -mcpu=SI -verify-machineinstrs < %s | FileCheck -check-prefix=SI %s
>  
>  ; SILowerI1Copies was not handling IMPLICIT_DEF
> -; SI-LABEL: @br_implicit_def
> +; SI-LABEL: {{^}}br_implicit_def:
>  ; SI: BB#0:
>  ; SI-NEXT: s_and_saveexec_b64
>  ; SI-NEXT: s_xor_b64
> diff --git a/test/CodeGen/R600/i1-copy-phi.ll b/test/CodeGen/R600/i1-copy-phi.ll
> index d987d73..bfa8672 100644
> --- a/test/CodeGen/R600/i1-copy-phi.ll
> +++ b/test/CodeGen/R600/i1-copy-phi.ll
> @@ -1,6 +1,14 @@
> -; XFAIL: *
> -; RUN: llc -march=r600 -mcpu=SI -verify-machineinstrs < %s
> +; RUN: llc -march=r600 -mcpu=SI -verify-machineinstrs < %s | FileCheck -check-prefix=SI %s
>  
> +; SI-LABEL: {{^}}br_i1_phi:
> +; SI: v_mov_b32_e32 [[REG:v[0-9]+]], 0{{$}}
> +; SI: s_and_saveexec_b64
> +; SI: s_xor_b64
> +; SI: v_mov_b32_e32 [[REG]], -1{{$}}
> +; SI: v_cmp_ne_i32_e64 {{s\[[0-9]+:[0-9]+\]}}, [[REG]], 0
> +; SI: s_and_saveexec_b64
> +; SI: s_xor_b64
> +; SI: s_endpgm
>  define void @br_i1_phi(i32 %arg, i1 %arg1) #0 {
>  bb:
>    br i1 %arg1, label %bb2, label %bb3
> diff --git a/test/CodeGen/R600/split-scalar-i64-add.ll b/test/CodeGen/R600/split-scalar-i64-add.ll
> index e9968af..e3448dc 100644
> --- a/test/CodeGen/R600/split-scalar-i64-add.ll
> +++ b/test/CodeGen/R600/split-scalar-i64-add.ll
> @@ -1,4 +1,3 @@
> -; XFAIL:*
>  ; RUN: llc -march=r600 -mcpu=SI -verify-machineinstrs < %s | FileCheck -check-prefix=SI -check-prefix=FUNC %s
>  
>  declare i32 @llvm.r600.read.tidig.x() readnone
> @@ -9,6 +8,8 @@ declare i32 @llvm.r600.read.tidig.x() readnone
>  ; scc instead.
>  
>  ; FUNC-LABEL: {{^}}imp_def_vcc_split_i64_add_0:
> +; SI: v_add_i32
> +; SI: v_addc_u32
>  define void @imp_def_vcc_split_i64_add_0(i64 addrspace(1)* %out, i32 %val) {
>    %vec.0 = insertelement <2 x i32> undef, i32 %val, i32 0
>    %vec.1 = insertelement <2 x i32> %vec.0, i32 999999, i32 1
> @@ -19,6 +20,8 @@ define void @imp_def_vcc_split_i64_add_0(i64 addrspace(1)* %out, i32 %val) {
>  }
>  
>  ; FUNC-LABEL: {{^}}imp_def_vcc_split_i64_add_1:
> +; SI: v_add_i32
> +; SI: v_addc_u32
>  define void @imp_def_vcc_split_i64_add_1(i64 addrspace(1)* %out, i32 %val0, i64 %val1) {
>    %vec.0 = insertelement <2 x i32> undef, i32 %val0, i32 0
>    %vec.1 = insertelement <2 x i32> %vec.0, i32 99999, i32 1
> @@ -30,6 +33,8 @@ define void @imp_def_vcc_split_i64_add_1(i64 addrspace(1)* %out, i32 %val0, i64
>  
>  ; Doesn't use constants
>  ; FUNC-LABEL @imp_def_vcc_split_i64_add_2
> +; SI: v_add_i32
> +; SI: v_addc_u32
>  define void @imp_def_vcc_split_i64_add_2(i64 addrspace(1)* %out, i32 addrspace(1)* %in, i32 %val0, i64 %val1) {
>    %tid = call i32 @llvm.r600.read.tidig.x() readnone
>    %gep = getelementptr i32 addrspace(1)* %in, i32 %tid
> -- 
> 2.1.2
> 

> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-R600-SI-Make-sure-resource-descriptors-are-always-st.patch
Type: text/x-diff
Size: 4422 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141117/9cfe900b/attachment.patch>

More information about the llvm-commits mailing list