[PATCH] R600/SI: Move SIFixSGPRCopies to right after isel
Matt Arsenault
Matthew.Arsenault at amd.com
Tue Nov 18 13:09:52 PST 2014
On 11/17/2014 12:07 PM, Tom Stellard wrote:
> 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
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
r222174, 222175, 222256
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141118/6fb70eee/attachment.html>
More information about the llvm-commits
mailing list