From a9a58fa236ab19b5caae32330d31e30ebdf6751f Mon Sep 17 00:00:00 2001 From: Marek Olsak Date: Tue, 10 Apr 2018 22:48:23 +0000 Subject: [PATCH] AMDGPU: enable 128-bit for local addr space under an option Author: Samuel Pitoiset ds_read_b128 and ds_write_b128 have been recently enabled under the amdgpu-ds128 option because the performance benefit is unclear. Though, using 128-bit loads/stores for the local address space appears to introduce regressions in tessellation shaders. Not sure what is broken, but as ds_read_b128/ds_write_b128 are not enabled by default, just introduce a global option and enable 128-bit only if requested (until it's fixed/used correctly). v2: - fix regressions in merge-stores.ll and multiple_tails.ll Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105464 llvm-svn: 329764 --- llvm/lib/Target/AMDGPU/AMDGPU.td | 6 ++++++ llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp | 1 + llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h | 5 +++-- llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp | 8 +++++--- llvm/lib/Target/AMDGPU/SIISelLowering.cpp | 9 ++------- llvm/test/CodeGen/AMDGPU/load-local-f32.ll | 6 +++--- llvm/test/CodeGen/AMDGPU/load-local-f64.ll | 4 ++-- llvm/test/CodeGen/AMDGPU/load-local-i16.ll | 4 ++-- llvm/test/CodeGen/AMDGPU/load-local-i32.ll | 6 +++--- llvm/test/CodeGen/AMDGPU/load-local-i64.ll | 4 ++-- llvm/test/CodeGen/AMDGPU/load-local-i8.ll | 4 ++-- llvm/test/Transforms/LoadStoreVectorizer/AMDGPU/merge-stores.ll | 3 ++- .../test/Transforms/LoadStoreVectorizer/AMDGPU/multiple_tails.ll | 3 ++- 13 files changed, 35 insertions(+), 28 deletions(-) diff --git a/llvm/lib/Target/AMDGPU/AMDGPU.td b/llvm/lib/Target/AMDGPU/AMDGPU.td index ff6baf7..b419334 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPU.td +++ b/llvm/lib/Target/AMDGPU/AMDGPU.td @@ -426,6 +426,12 @@ def FeatureEnableSIScheduler : SubtargetFeature<"si-scheduler", "Enable SI Machine Scheduler" >; +def FeatureEnableDS128 : SubtargetFeature<"enable-ds128", + "EnableDS128", + "true", + "Use ds_{read|write}_b128" +>; + // Unless +-flat-for-global is specified, turn on FlatForGlobal for // all OS-es on VI and newer hardware to avoid assertion failures due // to missing ADDR64 variants of MUBUF instructions. diff --git a/llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp b/llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp index 27b5799..07ed04e 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp +++ b/llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp @@ -132,6 +132,7 @@ AMDGPUSubtarget::AMDGPUSubtarget(const Triple &TT, StringRef GPU, StringRef FS, EnableLoadStoreOpt(false), EnableUnsafeDSOffsetFolding(false), EnableSIScheduler(false), + EnableDS128(false), DumpCode(false), FP64(false), diff --git a/llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h b/llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h index e3455d3..cd08026 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h +++ b/llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h @@ -133,6 +133,7 @@ protected: bool EnableLoadStoreOpt; bool EnableUnsafeDSOffsetFolding; bool EnableSIScheduler; + bool EnableDS128; bool DumpCode; // Subtarget statically properties set by tablegen @@ -412,8 +413,8 @@ public: /// \returns If target supports ds_read/write_b128 and user enables generation /// of ds_read/write_b128. - bool useDS128(bool UserEnable) const { - return CIInsts && UserEnable; + bool useDS128() const { + return CIInsts && EnableDS128; } /// \returns If MUBUF instructions always perform range checking, even for diff --git a/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp b/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp index 9f097cd..bf560a9 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp +++ b/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp @@ -265,11 +265,13 @@ unsigned AMDGPUTTIImpl::getLoadStoreVecRegBitWidth(unsigned AddrSpace) const { return 512; } - if (AddrSpace == AS.FLAT_ADDRESS || - AddrSpace == AS.LOCAL_ADDRESS || - AddrSpace == AS.REGION_ADDRESS) + if (AddrSpace == AS.FLAT_ADDRESS) return 128; + if (AddrSpace == AS.LOCAL_ADDRESS || + AddrSpace == AS.REGION_ADDRESS) + return ST->useDS128() ? 128 : 64; + if (AddrSpace == AS.PRIVATE_ADDRESS) return 8 * ST->getMaxPrivateElementSize(); diff --git a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp index 6f68f63..0829962 100644 --- a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp +++ b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp @@ -94,11 +94,6 @@ static cl::opt EnableVGPRIndexMode( cl::desc("Use GPR indexing mode instead of movrel for vector indexing"), cl::init(false)); -static cl::opt EnableDS128( - "amdgpu-ds128", - cl::desc("Use DS_read/write_b128"), - cl::init(false)); - static cl::opt AssumeFrameIndexHighZeroBits( "amdgpu-frame-index-zero-bits", cl::desc("High bits of frame index assumed to be zero"), @@ -5300,7 +5295,7 @@ SDValue SITargetLowering::LowerLOAD(SDValue Op, SelectionDAG &DAG) const { } } else if (AS == AMDGPUASI.LOCAL_ADDRESS) { // Use ds_read_b128 if possible. - if (Subtarget->useDS128(EnableDS128) && Load->getAlignment() >= 16 && + if (Subtarget->useDS128() && Load->getAlignment() >= 16 && MemVT.getStoreSize() == 16) return SDValue(); @@ -5703,7 +5698,7 @@ SDValue SITargetLowering::LowerSTORE(SDValue Op, SelectionDAG &DAG) const { } } else if (AS == AMDGPUASI.LOCAL_ADDRESS) { // Use ds_write_b128 if possible. - if (Subtarget->useDS128(EnableDS128) && Store->getAlignment() >= 16 && + if (Subtarget->useDS128() && Store->getAlignment() >= 16 && VT.getStoreSize() == 16) return SDValue(); diff --git a/llvm/test/CodeGen/AMDGPU/load-local-f32.ll b/llvm/test/CodeGen/AMDGPU/load-local-f32.ll index c272271..c33c000 100644 --- a/llvm/test/CodeGen/AMDGPU/load-local-f32.ll +++ b/llvm/test/CodeGen/AMDGPU/load-local-f32.ll @@ -3,9 +3,9 @@ ; RUN: llc -march=r600 -mcpu=redwood < %s | FileCheck -check-prefixes=EG,FUNC %s ; Testing for ds_read/write_128 -; RUN: llc -march=amdgcn -mcpu=tahiti -amdgpu-ds128 < %s | FileCheck -check-prefixes=SI,FUNC %s -; RUN: llc -march=amdgcn -mcpu=tonga -amdgpu-ds128 < %s | FileCheck -check-prefixes=CIVI,FUNC %s -; RUN: llc -march=amdgcn -mcpu=gfx900 -amdgpu-ds128 < %s | FileCheck -check-prefixes=CIVI,FUNC %s +; RUN: llc -march=amdgcn -mcpu=tahiti -mattr=+enable-ds128 < %s | FileCheck -check-prefixes=SI,FUNC %s +; RUN: llc -march=amdgcn -mcpu=tonga -mattr=+enable-ds128 < %s | FileCheck -check-prefixes=CIVI,FUNC %s +; RUN: llc -march=amdgcn -mcpu=gfx900 -mattr=+enable-ds128 < %s | FileCheck -check-prefixes=CIVI,FUNC %s ; FUNC-LABEL: {{^}}load_f32_local: ; SICIVI: s_mov_b32 m0 diff --git a/llvm/test/CodeGen/AMDGPU/load-local-f64.ll b/llvm/test/CodeGen/AMDGPU/load-local-f64.ll index f4040db..e313b38 100644 --- a/llvm/test/CodeGen/AMDGPU/load-local-f64.ll +++ b/llvm/test/CodeGen/AMDGPU/load-local-f64.ll @@ -5,8 +5,8 @@ ; RUN: llc -march=r600 -mcpu=redwood < %s | FileCheck -check-prefixes=EG,FUNC %s ; Testing for ds_read_b128 -; RUN: llc -march=amdgcn -mcpu=tonga -verify-machineinstrs -amdgpu-ds128 < %s | FileCheck -check-prefixes=CIVI,FUNC %s -; RUN: llc -march=amdgcn -mcpu=gfx900 -verify-machineinstrs -amdgpu-ds128 < %s | FileCheck -check-prefixes=CIVI,FUNC %s +; RUN: llc -march=amdgcn -mcpu=tonga -verify-machineinstrs -mattr=+enable-ds128 < %s | FileCheck -check-prefixes=CIVI,FUNC %s +; RUN: llc -march=amdgcn -mcpu=gfx900 -verify-machineinstrs -mattr=+enable-ds128 < %s | FileCheck -check-prefixes=CIVI,FUNC %s ; FUNC-LABEL: {{^}}local_load_f64: ; SICIV: s_mov_b32 m0 diff --git a/llvm/test/CodeGen/AMDGPU/load-local-i16.ll b/llvm/test/CodeGen/AMDGPU/load-local-i16.ll index 83cf85b..42beb22 100644 --- a/llvm/test/CodeGen/AMDGPU/load-local-i16.ll +++ b/llvm/test/CodeGen/AMDGPU/load-local-i16.ll @@ -4,8 +4,8 @@ ; RUN: llc -march=r600 -mcpu=redwood -verify-machineinstrs < %s | FileCheck -check-prefix=EG -check-prefix=FUNC %s ; Testing for ds_read/write_b128 -; RUN: llc -march=amdgcn -mcpu=tonga -amdgpu-ds128 < %s | FileCheck -check-prefixes=CIVI,FUNC %s -; RUN: llc -march=amdgcn -mcpu=gfx900 -amdgpu-ds128 < %s | FileCheck -check-prefixes=CIVI,FUNC %s +; RUN: llc -march=amdgcn -mcpu=tonga -mattr=+enable-ds128 < %s | FileCheck -check-prefixes=CIVI,FUNC %s +; RUN: llc -march=amdgcn -mcpu=gfx900 -mattr=+enable-ds128 < %s | FileCheck -check-prefixes=CIVI,FUNC %s ; FUNC-LABEL: {{^}}local_load_i16: ; GFX9-NOT: m0 diff --git a/llvm/test/CodeGen/AMDGPU/load-local-i32.ll b/llvm/test/CodeGen/AMDGPU/load-local-i32.ll index 2d0e989..fd2a739 100644 --- a/llvm/test/CodeGen/AMDGPU/load-local-i32.ll +++ b/llvm/test/CodeGen/AMDGPU/load-local-i32.ll @@ -4,9 +4,9 @@ ; RUN: llc -march=r600 -mcpu=redwood < %s | FileCheck -check-prefix=EG -check-prefix=FUNC %s ; Testing for ds_read/write_128 -; RUN: llc -march=amdgcn -mcpu=tahiti -amdgpu-ds128 < %s | FileCheck -check-prefixes=SI,FUNC %s -; RUN: llc -march=amdgcn -mcpu=tonga -amdgpu-ds128 < %s | FileCheck -check-prefixes=CIVI,FUNC %s -; RUN: llc -march=amdgcn -mcpu=gfx900 -amdgpu-ds128 < %s | FileCheck -check-prefixes=CIVI,FUNC %s +; RUN: llc -march=amdgcn -mcpu=tahiti -mattr=+enable-ds128 < %s | FileCheck -check-prefixes=SI,FUNC %s +; RUN: llc -march=amdgcn -mcpu=tonga -mattr=+enable-ds128 < %s | FileCheck -check-prefixes=CIVI,FUNC %s +; RUN: llc -march=amdgcn -mcpu=gfx900 -mattr=+enable-ds128 < %s | FileCheck -check-prefixes=CIVI,FUNC %s ; FUNC-LABEL: {{^}}local_load_i32: ; GCN-NOT: s_wqm_b64 diff --git a/llvm/test/CodeGen/AMDGPU/load-local-i64.ll b/llvm/test/CodeGen/AMDGPU/load-local-i64.ll index 697f474..d91c3b5 100644 --- a/llvm/test/CodeGen/AMDGPU/load-local-i64.ll +++ b/llvm/test/CodeGen/AMDGPU/load-local-i64.ll @@ -5,8 +5,8 @@ ; RUN: llc -march=r600 -mcpu=redwood < %s | FileCheck -check-prefixes=EG,FUNC %s ; Testing for ds_read/write_b128 -; RUN: llc -march=amdgcn -mcpu=tonga -verify-machineinstrs -amdgpu-ds128 < %s | FileCheck -check-prefixes=CIVI,FUNC %s -; RUN: llc -march=amdgcn -mcpu=gfx900 -verify-machineinstrs -amdgpu-ds128 < %s | FileCheck -check-prefixes=CIVI,FUNC %s +; RUN: llc -march=amdgcn -mcpu=tonga -verify-machineinstrs -mattr=+enable-ds128 < %s | FileCheck -check-prefixes=CIVI,FUNC %s +; RUN: llc -march=amdgcn -mcpu=gfx900 -verify-machineinstrs -mattr=+enable-ds128 < %s | FileCheck -check-prefixes=CIVI,FUNC %s ; FUNC-LABEL: {{^}}local_load_i64: ; SICIVI: s_mov_b32 m0 diff --git a/llvm/test/CodeGen/AMDGPU/load-local-i8.ll b/llvm/test/CodeGen/AMDGPU/load-local-i8.ll index 898d35d..81e5ceb 100644 --- a/llvm/test/CodeGen/AMDGPU/load-local-i8.ll +++ b/llvm/test/CodeGen/AMDGPU/load-local-i8.ll @@ -4,8 +4,8 @@ ; RUN: llc -march=r600 -mtriple=r600---amdgiz -mcpu=redwood -verify-machineinstrs < %s | FileCheck -check-prefix=EG -check-prefix=FUNC %s ; Testing for ds_read/write_b128 -; RUN: llc -march=amdgcn -mcpu=tonga -amdgpu-ds128 < %s | FileCheck -check-prefixes=CIVI,FUNC %s -; RUN: llc -march=amdgcn -mcpu=gfx900 -amdgpu-ds128 < %s | FileCheck -check-prefixes=CIVI,FUNC %s +; RUN: llc -march=amdgcn -mcpu=tonga -mattr=+enable-ds128 < %s | FileCheck -check-prefixes=CIVI,FUNC %s +; RUN: llc -march=amdgcn -mcpu=gfx900 -mattr=+enable-ds128 < %s | FileCheck -check-prefixes=CIVI,FUNC %s ; FUNC-LABEL: {{^}}local_load_i8: ; GCN-NOT: s_wqm_b64 diff --git a/llvm/test/Transforms/LoadStoreVectorizer/AMDGPU/merge-stores.ll b/llvm/test/Transforms/LoadStoreVectorizer/AMDGPU/merge-stores.ll index 5eb3b25..19fc44b 100644 --- a/llvm/test/Transforms/LoadStoreVectorizer/AMDGPU/merge-stores.ll +++ b/llvm/test/Transforms/LoadStoreVectorizer/AMDGPU/merge-stores.ll @@ -504,7 +504,8 @@ define amdgpu_kernel void @merge_local_store_2_constants_i32_align_2(i32 addrspa } ; CHECK-LABEL: @merge_local_store_4_constants_i32 -; CHECK: store <4 x i32> , <4 x i32> addrspace(3)* +; CHECK: store <2 x i32> , <2 x i32> addrspace(3)* +; CHECK: store <2 x i32> , <2 x i32> addrspace(3)* define amdgpu_kernel void @merge_local_store_4_constants_i32(i32 addrspace(3)* %out) #0 { %out.gep.1 = getelementptr i32, i32 addrspace(3)* %out, i32 1 %out.gep.2 = getelementptr i32, i32 addrspace(3)* %out, i32 2 diff --git a/llvm/test/Transforms/LoadStoreVectorizer/AMDGPU/multiple_tails.ll b/llvm/test/Transforms/LoadStoreVectorizer/AMDGPU/multiple_tails.ll index b684ca8..8a78f3d 100644 --- a/llvm/test/Transforms/LoadStoreVectorizer/AMDGPU/multiple_tails.ll +++ b/llvm/test/Transforms/LoadStoreVectorizer/AMDGPU/multiple_tails.ll @@ -29,10 +29,11 @@ define amdgpu_kernel void @no_crash(i32 %arg) { ; longest chain vectorized ; CHECK-LABEL: @interleave_get_longest -; CHECK: load <4 x i32> +; CHECK: load <2 x i32> ; CHECK: load i32 ; CHECK: store <2 x i32> zeroinitializer ; CHECK: load i32 +; CHECK: load <2 x i32> ; CHECK: load i32 ; CHECK: load i32 -- 2.7.4