From ccc1bbe6b1e96cb83e4bf328133f3fbc877b6b16 Mon Sep 17 00:00:00 2001 From: Ben Sander Date: Sun, 11 Sep 2016 11:02:33 -0500 Subject: [PATCH] Remove HIP_STAGING_BUFFER Code simplification/cleanup: Remove stale fallback paths that uses something besides the unpinned engine. Remove HIP_STAGING_BUFFER env var - now is const 2, 0 no longer has special meaning. Change-Id: I7d24cdd1067dd0c244e87b6a83897cb135d307e7 --- include/hcc_detail/hip_hcc.h | 1 - src/hip_hcc.cpp | 86 ++++++++++++------------------------ 2 files changed, 29 insertions(+), 58 deletions(-) diff --git a/include/hcc_detail/hip_hcc.h b/include/hcc_detail/hip_hcc.h index 94df169e07..3ce2e7ed2d 100644 --- a/include/hcc_detail/hip_hcc.h +++ b/include/hcc_detail/hip_hcc.h @@ -55,7 +55,6 @@ extern int HIP_ATP_MARKER; extern int HIP_ATP; extern int HIP_DB; extern int HIP_STAGING_SIZE; /* size of staging buffers, in KB */ -extern int HIP_STAGING_BUFFERS; // TODO - remove, two buffers should be enough. extern int HIP_PININPLACE; extern int HIP_STREAM_SIGNALS; /* number of signals to allocate at stream creation */ extern int HIP_VISIBLE_DEVICES; /* Contains a comma-separated sequence of GPU identifiers */ diff --git a/src/hip_hcc.cpp b/src/hip_hcc.cpp index 294f8d8e89..09fbe545cc 100644 --- a/src/hip_hcc.cpp +++ b/src/hip_hcc.cpp @@ -68,7 +68,7 @@ std::string HIP_TRACE_API_COLOR("green"); int HIP_ATP_MARKER= 0; int HIP_DB= 0; int HIP_STAGING_SIZE = 64; /* size of staging buffers, in KB */ -int HIP_STAGING_BUFFERS = 2; // TODO - remove, two buffers should be enough. +static const int HIP_STAGING_BUFFERS = 2; int HIP_PININPLACE = 0; int HIP_OPTIMAL_MEM_TRANSFER = 0; //ENV Variable to test different memory transfer logics int HIP_H2D_MEM_TRANSFER_THRESHOLD_DIRECT_OR_STAGING = 0; @@ -1279,7 +1279,6 @@ void ihipInit() READ_ENV_S(release, HIP_TRACE_API_COLOR, 0, "Color to use for HIP_API. None/Red/Green/Yellow/Blue/Magenta/Cyan/White"); READ_ENV_I(release, HIP_ATP_MARKER, 0, "Add HIP function begin/end to ATP file generated with CodeXL"); READ_ENV_I(release, HIP_STAGING_SIZE, 0, "Size of each staging buffer (in KB)" ); - READ_ENV_I(release, HIP_STAGING_BUFFERS, 0, "Number of staging buffers to use in each direction. 0=use hsa_memory_copy."); READ_ENV_I(release, HIP_PININPLACE, 0, "For unpinned transfers, pin the memory in-place in chunks before doing the copy."); READ_ENV_I(release, HIP_OPTIMAL_MEM_TRANSFER, 0, "For optimal memory transfers for unpinned memory.Under testing."); READ_ENV_I(release, HIP_H2D_MEM_TRANSFER_THRESHOLD_DIRECT_OR_STAGING, 0, "Threshold value for H2D unpinned memory transfer decision between direct copy or staging buffer usage,Under testing."); @@ -1744,28 +1743,17 @@ void ihipStream_t::copySync(LockedAccessor_StreamCrit_t &crit, void* dst, const if (kind == hipMemcpyHostToDevice) { int depSignalCnt = preCopyCommand(crit, NULL, &depSignal, ihipCommandCopyH2D); if(!srcTracked){ - if (HIP_STAGING_BUFFERS) { - tprintf(DB_COPY1, "D2H && !dstTracked: staged copy H2D dst=%p src=%p sz=%zu\n", dst, src, sizeBytes); - UnpinnedCopyEngine::CopyMode copyMode = UnpinnedCopyEngine::UseStaging; + tprintf(DB_COPY1, "D2H && !dstTracked: staged copy H2D dst=%p src=%p sz=%zu\n", dst, src, sizeBytes); + UnpinnedCopyEngine::CopyMode copyMode = UnpinnedCopyEngine::UseStaging; - if (HIP_OPTIMAL_MEM_TRANSFER) { - copyMode = UnpinnedCopyEngine::ChooseBest; - } else if (HIP_PININPLACE) { - copyMode = UnpinnedCopyEngine::UsePinInPlace; - } - device->_stagingBuffer[0]->CopyHostToDevice(copyMode, dst, src, sizeBytes, depSignalCnt ? &depSignal : NULL); - // The copy waits for inputs and then completes before returning so can reset queue to empty: - this->wait(crit, true); - } - else { - // TODO - remove, slow path. - tprintf(DB_COPY1, "H2D && ! srcTracked: am_copy dst=%p src=%p sz=%zu\n", dst, src, sizeBytes); -#if USE_AV_COPY - crit->_av.copy(src,dst,sizeBytes); -#else - hc::am_copy(dst, src, sizeBytes); -#endif + if (HIP_OPTIMAL_MEM_TRANSFER) { + copyMode = UnpinnedCopyEngine::ChooseBest; + } else if (HIP_PININPLACE) { + copyMode = UnpinnedCopyEngine::UsePinInPlace; } + device->_stagingBuffer[0]->CopyHostToDevice(copyMode, dst, src, sizeBytes, depSignalCnt ? &depSignal : NULL); + // The copy waits for inputs and then completes before returning so can reset queue to empty: + this->wait(crit, true); } else { // This is H2D copy, and source is pinned host memory : we can copy directly w/o using staging buffer. hsa_agent_t dstAgent = *(static_cast(dstPtrInfo._acc.get_hsa_agent())); @@ -1790,30 +1778,19 @@ void ihipStream_t::copySync(LockedAccessor_StreamCrit_t &crit, void* dst, const } else if (kind == hipMemcpyDeviceToHost) { int depSignalCnt = preCopyCommand(crit, NULL, &depSignal, ihipCommandCopyD2H); if (!dstTracked){ - if (HIP_STAGING_BUFFERS) { - tprintf(DB_COPY1, "D2H && !dstTracked: staged copy D2H dst=%p src=%p sz=%zu\n", dst, src, sizeBytes); - UnpinnedCopyEngine::CopyMode copyMode = UnpinnedCopyEngine::UseStaging; + tprintf(DB_COPY1, "D2H && !dstTracked: staged copy D2H dst=%p src=%p sz=%zu\n", dst, src, sizeBytes); + UnpinnedCopyEngine::CopyMode copyMode = UnpinnedCopyEngine::UseStaging; - if (HIP_OPTIMAL_MEM_TRANSFER) { - copyMode = UnpinnedCopyEngine::ChooseBest; - } else if (HIP_PININPLACE) { - copyMode = UnpinnedCopyEngine::UsePinInPlace; - } - - device->_stagingBuffer[1]->CopyDeviceToHost(copyMode, dst, src, sizeBytes, depSignalCnt ? &depSignal : NULL); - - // The copy completes before returning so can reset queue to empty: - this->wait(crit, true); - - } else { - // TODO - remove, slow path. - tprintf(DB_COPY1, "D2H && !dstTracked: am_copy dst=%p src=%p sz=%zu\n", dst, src, sizeBytes); -#if USE_AV_COPY - crit->_av.copy(src, dst, sizeBytes); -#else - hc::am_copy(dst, src, sizeBytes); -#endif + if (HIP_OPTIMAL_MEM_TRANSFER) { + copyMode = UnpinnedCopyEngine::ChooseBest; + } else if (HIP_PININPLACE) { + copyMode = UnpinnedCopyEngine::UsePinInPlace; } + + device->_stagingBuffer[1]->CopyDeviceToHost(copyMode, dst, src, sizeBytes, depSignalCnt ? &depSignal : NULL); + + // The copy completes before returning so can reset queue to empty: + this->wait(crit, true); } else { // This is D2H copy, and destination is pinned host memory : we can copy directly w/o using staging buffer. hsa_agent_t dstAgent = *(static_cast(dstPtrInfo._acc.get_hsa_agent())); @@ -1845,21 +1822,15 @@ void ihipStream_t::copySync(LockedAccessor_StreamCrit_t &crit, void* dst, const memcpy(dst, src, sizeBytes); } else if ((kind == hipMemcpyDeviceToDevice) && !copyEngineCanSeeSrcAndDest) { int depSignalCnt = preCopyCommand(crit, NULL, &depSignal, ihipCommandCopyP2P); - if (HIP_STAGING_BUFFERS) { - tprintf(DB_COPY1, "P2P but engine can't see both pointers: staged copy P2P dst=%p src=%p sz=%zu\n", dst, src, sizeBytes); - //printf ("staged-copy- read dep signals\n"); - hsa_agent_t dstAgent = * (static_cast (dstPtrInfo._acc.get_hsa_agent())); - hsa_agent_t srcAgent = * (static_cast (srcPtrInfo._acc.get_hsa_agent())); + tprintf(DB_COPY1, "P2P but engine can't see both pointers: staged copy P2P dst=%p src=%p sz=%zu\n", dst, src, sizeBytes); + //printf ("staged-copy- read dep signals\n"); + hsa_agent_t dstAgent = * (static_cast (dstPtrInfo._acc.get_hsa_agent())); + hsa_agent_t srcAgent = * (static_cast (srcPtrInfo._acc.get_hsa_agent())); - device->_stagingBuffer[1]->CopyPeerToPeer(dst, dstAgent, src, srcAgent, sizeBytes, depSignalCnt ? &depSignal : NULL); - - // The copy completes before returning so can reset queue to empty: - this->wait(crit, true); - - } else { - assert(0); // currently no fallback for this path. - } + device->_stagingBuffer[1]->CopyPeerToPeer(dst, dstAgent, src, srcAgent, sizeBytes, depSignalCnt ? &depSignal : NULL); + // The copy completes before returning so can reset queue to empty: + this->wait(crit, true); } else { // If not special case - these can all be handled by the hsa async copy: ihipCommand_t commandType; @@ -1896,6 +1867,7 @@ void ihipStream_t::locked_copySync(void* dst, const void* src, size_t sizeBytes, copySync(crit, dst, src, sizeBytes, kind, resolveOn); } + void ihipStream_t::copyAsync(void* dst, const void* src, size_t sizeBytes, unsigned kind) { LockedAccessor_StreamCrit_t crit(_criticalData);