Skip to content

Commit

Permalink
Revert "Use result errors for resource creation. (#911)"
Browse files Browse the repository at this point in the history 
This reverts commit a10bfda.
  • Loading branch information
@bbernhar
bbernhar committed Aug 18, 2023
1 parent a10bfda commit 3c22362
Show file tree
Hide file tree
Showing 11 changed files with 136 additions and 180 deletions.
4 changes: 0 additions & 4 deletions src/gpgmm/common/Error.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,6 @@ namespace gpgmm {
return *this;
}

ErrorCode GetErrorCode() const {
return mErrorCode;
}

bool IsSuccess() const {
return mErrorCode == ErrorCode::kNone;
}
Expand Down
23 changes: 11 additions & 12 deletions src/gpgmm/d3d12/CapsD3D12.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ namespace gpgmm::d3d12 {

HRESULT SetMaxResourceSize(ID3D12Device* device, uint64_t* sizeOut) {
D3D12_FEATURE_DATA_GPU_VIRTUAL_ADDRESS_SUPPORT feature = {};
GPGMM_RETURN_RESULT_IF_FAILED(
GPGMM_RETURN_IF_FAILED(
device->CheckFeatureSupport(D3D12_FEATURE_GPU_VIRTUAL_ADDRESS_SUPPORT, &feature,
sizeof(D3D12_FEATURE_DATA_GPU_VIRTUAL_ADDRESS_SUPPORT)),
device);
Expand All @@ -43,7 +43,7 @@ namespace gpgmm::d3d12 {

HRESULT SetMaxResourceHeapSize(ID3D12Device* device, uint64_t* sizeOut) {
D3D12_FEATURE_DATA_GPU_VIRTUAL_ADDRESS_SUPPORT feature = {};
GPGMM_RETURN_RESULT_IF_FAILED(
GPGMM_RETURN_IF_FAILED(
device->CheckFeatureSupport(D3D12_FEATURE_GPU_VIRTUAL_ADDRESS_SUPPORT, &feature,
sizeof(D3D12_FEATURE_DATA_GPU_VIRTUAL_ADDRESS_SUPPORT)),
device);
Expand Down Expand Up @@ -76,7 +76,7 @@ namespace gpgmm::d3d12 {
HRESULT SetMaxResourceHeapTierSupported(ID3D12Device* device,
D3D12_RESOURCE_HEAP_TIER* maxResourceHeapTierOut) {
D3D12_FEATURE_DATA_D3D12_OPTIONS options = {};
GPGMM_RETURN_RESULT_IF_FAILED(
GPGMM_RETURN_IF_FAILED(
device->CheckFeatureSupport(D3D12_FEATURE_D3D12_OPTIONS, &options, sizeof(options)),
device);
*maxResourceHeapTierOut = options.ResourceHeapTier;
Expand All @@ -85,27 +85,26 @@ namespace gpgmm::d3d12 {

// static
HRESULT Caps::CreateCaps(ID3D12Device* device, IDXGIAdapter* adapter, Caps** capsOut) {
GPGMM_RETURN_RESULT_IF_NULLPTR(device);
GPGMM_RETURN_IF_NULLPTR(device);

std::unique_ptr<Caps> caps(new Caps());
GPGMM_RETURN_RESULT_IF_FAILED(SetMaxResourceSize(device, &caps->mMaxResourceSize), device);
GPGMM_RETURN_RESULT_IF_FAILED(SetMaxResourceHeapSize(device, &caps->mMaxResourceHeapSize),
device);
GPGMM_RETURN_RESULT_IF_FAILED(
SetMaxResourceHeapTierSupported(device, &caps->mMaxResourceHeapTier), device);
GPGMM_RETURN_RESULT_IF_FAILED(
GPGMM_RETURN_IF_FAILED(SetMaxResourceSize(device, &caps->mMaxResourceSize), device);
GPGMM_RETURN_IF_FAILED(SetMaxResourceHeapSize(device, &caps->mMaxResourceHeapSize), device);
GPGMM_RETURN_IF_FAILED(SetMaxResourceHeapTierSupported(device, &caps->mMaxResourceHeapTier),
device);
GPGMM_RETURN_IF_FAILED(
SetCreateHeapNotResidentSupported(device, &caps->mIsCreateHeapNotResidentSupported),
device);

D3D12_FEATURE_DATA_ARCHITECTURE arch = {};
GPGMM_RETURN_RESULT_IF_FAILED(
GPGMM_RETURN_IF_FAILED(
device->CheckFeatureSupport(D3D12_FEATURE_ARCHITECTURE, &arch, sizeof(arch)), device);
caps->mIsAdapterUMA = arch.UMA;
caps->mIsAdapterCacheCoherentUMA = arch.CacheCoherentUMA;

if (adapter != nullptr) {
DXGI_ADAPTER_DESC adapterDesc;
GPGMM_RETURN_RESULT_IF_FAILED(adapter->GetDesc(&adapterDesc), device);
GPGMM_RETURN_IF_FAILED(adapter->GetDesc(&adapterDesc), device);

caps->mSharedSegmentSize = adapterDesc.SharedSystemMemory;
caps->mDedicatedSegmentSize = adapterDesc.DedicatedVideoMemory;
Expand Down
4 changes: 0 additions & 4 deletions src/gpgmm/d3d12/ErrorD3D12.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ namespace gpgmm::d3d12 {

ErrorCode GetErrorCode(HRESULT error) {
switch (error) {
case S_OK:
return ErrorCode::kNone;
case E_INVALIDARG:
case E_POINTER:
return ErrorCode::kInvalidArgument;
Expand All @@ -42,8 +40,6 @@ namespace gpgmm::d3d12 {

HRESULT GetErrorResult(ErrorCode error) {
switch (error) {
case ErrorCode::kNone:
return S_OK;
case ErrorCode::kInvalidArgument:
return E_INVALIDARG;
case ErrorCode::kBadOperation:
Expand Down
38 changes: 7 additions & 31 deletions src/gpgmm/d3d12/ErrorD3D12.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@

#include <string>

#define GPGMM_RETURN_RESULT_IF_NULLPTR(ptr) \
GPGMM_RETURN_RESULT_IF_FAILED((ptr == nullptr ? E_POINTER : S_OK), nullptr)
#define GPGMM_RETURN_IF_NULLPTR(ptr) \
GPGMM_RETURN_IF_FAILED((ptr == nullptr ? E_POINTER : S_OK), nullptr)

#define GPGMM_RETURN_RESULT_IF_FAILED(expr, device) \
#define GPGMM_RETURN_IF_FAILED(expr, device) \
{ \
auto GPGMM_LOCAL_VAR(HRESULT) = expr; \
if (GPGMM_UNLIKELY(FAILED(GPGMM_LOCAL_VAR(HRESULT)))) { \
Expand All @@ -38,31 +38,7 @@
for (;;) \
break

#define GPGMM_RETURN_ERROR_IF_FAILED(expr, device) \
{ \
auto GPGMM_LOCAL_VAR(HRESULT) = expr; \
if (GPGMM_UNLIKELY(FAILED(GPGMM_LOCAL_VAR(HRESULT)))) { \
gpgmm::ErrorLog(GetErrorCode(GPGMM_LOCAL_VAR(HRESULT))) \
<< #expr << ": " \
<< GetErrorResultWithRemovalReason(GPGMM_LOCAL_VAR(HRESULT), device); \
return GetErrorCode(GPGMM_LOCAL_VAR(HRESULT)); \
} \
} \
for (;;) \
break

#define GPGMM_RETURN_RESULT_IF_ERROR(expr) \
{ \
auto GPGMM_LOCAL_VAR(Result) = expr; \
if (GPGMM_UNLIKELY(!GPGMM_LOCAL_VAR(Result).IsSuccess())) { \
gpgmm::ErrorLog(GPGMM_LOCAL_VAR(Result).GetErrorCode()) << #expr; \
return GetErrorResult(GPGMM_LOCAL_VAR(Result).AcquireError()); \
} \
} \
for (;;) \
break

#define GPGMM_RETURN_RESULT_IF_SUCCEEDED(expr) \
#define GPGMM_RETURN_IF_SUCCEEDED(expr) \
{ \
auto GPGMM_LOCAL_VAR(HRESULT) = expr; \
if (GPGMM_LIKELY(SUCCEEDED(GPGMM_LOCAL_VAR(HRESULT)))) { \
Expand All @@ -72,14 +48,14 @@
for (;;) \
break

// Same as GPGMM_RETURN_RESULT_IF_SUCCEEDED but also returns if error is lethal.
// Same as GPGMM_RETURN_IF_SUCCEEDED but also returns if error is lethal.
// Non-internal errors are always fatal and should not run re-attempt logic.
#define GPGMM_RETURN_ERROR_IF_SUCCEEDED_OR_FATAL(expr) \
#define GPGMM_RETURN_IF_SUCCEEDED_OR_FATAL(expr) \
{ \
auto GPGMM_LOCAL_VAR(HRESULT) = expr; \
if (GPGMM_LIKELY(SUCCEEDED(GPGMM_LOCAL_VAR(HRESULT))) || \
GPGMM_UNLIKELY(IsErrorResultFatal(GPGMM_LOCAL_VAR(HRESULT)))) { \
return GetErrorCode(GPGMM_LOCAL_VAR(HRESULT)); \
return GPGMM_LOCAL_VAR(HRESULT); \
} \
} \
for (;;) \
Expand Down
11 changes: 5 additions & 6 deletions src/gpgmm/d3d12/FenceD3D12.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ namespace gpgmm::d3d12 {
// static
HRESULT Fence::CreateFence(ID3D12Device* device, uint64_t initialValue, Fence** fenceOut) {
ComPtr<ID3D12Fence> fence;
GPGMM_RETURN_RESULT_IF_FAILED(
GPGMM_RETURN_IF_FAILED(
device->CreateFence(initialValue, D3D12_FENCE_FLAG_NONE, IID_PPV_ARGS(&fence)), device);
*fenceOut = new Fence(fence, initialValue);
return S_OK;
Expand Down Expand Up @@ -53,9 +53,8 @@ namespace gpgmm::d3d12 {

HRESULT Fence::WaitFor(uint64_t fenceValue) {
if (!IsCompleted(fenceValue)) {
GPGMM_RETURN_RESULT_IF_FAILED(
mFence->SetEventOnCompletion(fenceValue, mCompletionEvent),
GetDevice(mFence.Get()));
GPGMM_RETURN_IF_FAILED(mFence->SetEventOnCompletion(fenceValue, mCompletionEvent),
GetDevice(mFence.Get()));

// Wait for the event to complete (it will automatically reset).
const uint32_t result = WaitForSingleObject(mCompletionEvent, INFINITE);
Expand Down Expand Up @@ -83,8 +82,8 @@ namespace gpgmm::d3d12 {

HRESULT Fence::Signal(ID3D12CommandQueue* pCommandQueue) {
ASSERT(mLastSignaledFence != mCurrentFence);
GPGMM_RETURN_RESULT_IF_FAILED(pCommandQueue->Signal(mFence.Get(), mCurrentFence),
GetDevice(pCommandQueue));
GPGMM_RETURN_IF_FAILED(pCommandQueue->Signal(mFence.Get(), mCurrentFence),
GetDevice(pCommandQueue));
mLastSignaledFence = mCurrentFence;
mCurrentFence++;
return S_OK;
Expand Down
30 changes: 14 additions & 16 deletions src/gpgmm/d3d12/ResidencyHeapD3D12.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,8 @@ namespace gpgmm::d3d12 {

ComPtr<ID3D12Resource> committedResource;
if (SUCCEEDED(pageable.As(&committedResource))) {
GPGMM_RETURN_RESULT_IF_FAILED(
committedResource->GetHeapProperties(nullptr, heapFlags),
GetDevice(committedResource.Get()));
GPGMM_RETURN_IF_FAILED(committedResource->GetHeapProperties(nullptr, heapFlags),
GetDevice(committedResource.Get()));
return S_OK;
}

Expand Down Expand Up @@ -133,7 +132,7 @@ namespace gpgmm::d3d12 {
"Heap.CreateResidencyHeap",
(RESIDENCY_HEAP_CREATE_RESIDENCY_HEAP_PARAMS{descriptor, pPageable}));

GPGMM_RETURN_RESULT_IF_NULLPTR(pPageable);
GPGMM_RETURN_IF_NULLPTR(pPageable);

ResidencyManager* residencyManager = static_cast<ResidencyManager*>(pResidencyManager);
const bool isResidencyDisabled = (pResidencyManager == nullptr);
Expand Down Expand Up @@ -188,14 +187,14 @@ namespace gpgmm::d3d12 {
// descriptor heap), they must be manually locked and unlocked to be inserted into the
// residency cache.
if (heap->GetInfo().Status != RESIDENCY_HEAP_STATUS_UNKNOWN) {
GPGMM_RETURN_RESULT_IF_FAILED(residencyManager->InsertHeap(heap.get()),
GetDevice(pPageable));
GPGMM_RETURN_IF_FAILED(residencyManager->InsertHeap(heap.get()),
GetDevice(pPageable));
} else {
if (newDescriptor.Flags & RESIDENCY_HEAP_FLAG_CREATE_RESIDENT) {
GPGMM_RETURN_RESULT_IF_FAILED(residencyManager->LockHeap(heap.get()),
GetDevice(pPageable));
GPGMM_RETURN_RESULT_IF_FAILED(residencyManager->UnlockHeap(heap.get()),
GetDevice(pPageable));
GPGMM_RETURN_IF_FAILED(residencyManager->LockHeap(heap.get()),
GetDevice(pPageable));
GPGMM_RETURN_IF_FAILED(residencyManager->UnlockHeap(heap.get()),
GetDevice(pPageable));
ASSERT(heap->GetInfo().Status == RESIDENCY_HEAP_STATUS_RESIDENT);
}
}
Expand All @@ -207,8 +206,7 @@ namespace gpgmm::d3d12 {
}
}

GPGMM_RETURN_RESULT_IF_FAILED(heap->SetDebugName(newDescriptor.DebugName),
GetDevice(pPageable));
GPGMM_RETURN_IF_FAILED(heap->SetDebugName(newDescriptor.DebugName), GetDevice(pPageable));
GPGMM_TRACE_EVENT_OBJECT_SNAPSHOT(heap.get(), newDescriptor);

DebugLog(MessageId::kObjectCreated, heap.get())
Expand All @@ -228,7 +226,7 @@ namespace gpgmm::d3d12 {
CreateHeapFn createHeapFn,
void* pCreateHeapContext,
IResidencyHeap** ppResidencyHeapOut) {
GPGMM_RETURN_RESULT_IF_NULLPTR(pCreateHeapContext);
GPGMM_RETURN_IF_NULLPTR(pCreateHeapContext);

const bool isResidencyDisabled = (pResidencyManager == nullptr);

Expand All @@ -250,7 +248,7 @@ namespace gpgmm::d3d12 {
// Ensure enough budget exists before creating the heap to avoid an out-of-memory error.
if (!isResidencyDisabled && (descriptor.Flags & RESIDENCY_HEAP_FLAG_CREATE_IN_BUDGET)) {
uint64_t bytesEvicted = descriptor.SizeInBytes;
GPGMM_RETURN_RESULT_IF_FAILED(
GPGMM_RETURN_IF_FAILED(
residencyManager->EvictInternal(descriptor.SizeInBytes, descriptor.HeapSegment,
&bytesEvicted),
residencyManager->mDevice);
Expand All @@ -273,8 +271,8 @@ namespace gpgmm::d3d12 {
}

ComPtr<ID3D12Pageable> pageable;
GPGMM_RETURN_RESULT_IF_FAILED(createHeapFn(pCreateHeapContext, &pageable),
GetDevice(pageable.Get()));
GPGMM_RETURN_IF_FAILED(createHeapFn(pCreateHeapContext, &pageable),
GetDevice(pageable.Get()));

return CreateResidencyHeap(descriptor, pResidencyManager, pageable.Get(),
ppResidencyHeapOut);
Expand Down
Loading

0 comments on commit 3c22362

Please sign in to comment.