Fix integer truncation bug

On Apple M1, maxStorageBufferBindingSize has all 0 in the lower 32 bits,
which was resulting in a value of 0 and causing an internal error.

Bug: dawn:1367
Change-Id: I00719051738f1ad87989dae0ddd96f1ddf325dab
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/87180
Reviewed-by: Loko Kung <lokokung@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>
This commit is contained in:
Aleksi Sapon 2022-04-18 22:18:05 +00:00 committed by Dawn LUCI CQ
parent c6be53e172
commit baeed1f72c
4 changed files with 19 additions and 11 deletions

View File

@ -25,7 +25,7 @@
namespace dawn::native {
uint32_t ComputeMaxIndirectValidationBatchOffsetRange(const CombinedLimits& limits) {
uint64_t ComputeMaxIndirectValidationBatchOffsetRange(const CombinedLimits& limits) {
return limits.v1.maxStorageBufferBindingSize - limits.v1.minStorageBufferOffsetAlignment -
kDrawIndexedIndirectSize;
}
@ -37,7 +37,7 @@ namespace dawn::native {
void IndirectDrawMetadata::IndexedIndirectBufferValidationInfo::AddIndexedIndirectDraw(
uint32_t maxDrawCallsPerIndirectValidationBatch,
uint32_t maxBatchOffsetRange,
uint64_t maxBatchOffsetRange,
IndexedIndirectDraw draw) {
const uint64_t newOffset = draw.clientBufferOffset;
auto it = mBatches.begin();
@ -92,7 +92,7 @@ namespace dawn::native {
void IndirectDrawMetadata::IndexedIndirectBufferValidationInfo::AddBatch(
uint32_t maxDrawCallsPerIndirectValidationBatch,
uint32_t maxBatchOffsetRange,
uint64_t maxBatchOffsetRange,
const IndexedIndirectValidationBatch& newBatch) {
auto it = mBatches.begin();
while (it != mBatches.end()) {
@ -123,8 +123,8 @@ namespace dawn::native {
}
IndirectDrawMetadata::IndirectDrawMetadata(const CombinedLimits& limits)
: mMaxDrawCallsPerBatch(ComputeMaxDrawCallsPerIndirectValidationBatch(limits)),
mMaxBatchOffsetRange(ComputeMaxIndirectValidationBatchOffsetRange(limits)) {
: mMaxBatchOffsetRange(ComputeMaxIndirectValidationBatchOffsetRange(limits)),
mMaxDrawCallsPerBatch(ComputeMaxDrawCallsPerIndirectValidationBatch(limits)) {
}
IndirectDrawMetadata::~IndirectDrawMetadata() = default;

View File

@ -34,7 +34,7 @@ namespace dawn::native {
// In the unlikely scenario that indirect offsets used over a single buffer span more than
// this length of the buffer, we split the validation work into multiple batches.
uint32_t ComputeMaxIndirectValidationBatchOffsetRange(const CombinedLimits& limits);
uint64_t ComputeMaxIndirectValidationBatchOffsetRange(const CombinedLimits& limits);
// Metadata corresponding to the validation requirements of a single render pass. This metadata
// is accumulated while its corresponding render pass is encoded, and is later used to encode
@ -66,14 +66,14 @@ namespace dawn::native {
// Logs a new drawIndexedIndirect call for the render pass. `cmd` is updated with an
// assigned (and deferred) buffer ref and relative offset before returning.
void AddIndexedIndirectDraw(uint32_t maxDrawCallsPerIndirectValidationBatch,
uint32_t maxBatchOffsetRange,
uint64_t maxBatchOffsetRange,
IndexedIndirectDraw draw);
// Adds draw calls from an already-computed batch, e.g. from a previously encoded
// RenderBundle. The added batch is merged into an existing batch if possible, otherwise
// it's added to mBatch.
void AddBatch(uint32_t maxDrawCallsPerIndirectValidationBatch,
uint32_t maxBatchOffsetRange,
uint64_t maxBatchOffsetRange,
const IndexedIndirectValidationBatch& batch);
const std::vector<IndexedIndirectValidationBatch>& GetBatches() const;
@ -117,8 +117,8 @@ namespace dawn::native {
IndexedIndirectBufferValidationInfoMap mIndexedIndirectBufferValidationInfo;
std::set<RenderBundleBase*> mAddedBundles;
uint64_t mMaxBatchOffsetRange;
uint32_t mMaxDrawCallsPerBatch;
uint32_t mMaxBatchOffsetRange;
};
} // namespace dawn::native

View File

@ -217,7 +217,7 @@ namespace dawn::native {
// single pass as possible. Batches can be grouped together as long as they're validating
// data from the same indirect buffer, but they may still be split into multiple passes if
// the number of draw calls in a pass would exceed some (very high) upper bound.
size_t validatedParamsSize = 0;
uint64_t validatedParamsSize = 0;
std::vector<Pass> passes;
IndirectDrawMetadata::IndexedIndirectBufferValidationInfoMap& bufferInfoMap =
*indirectDrawMetadata->GetIndexedIndirectBufferValidationInfo();
@ -225,7 +225,7 @@ namespace dawn::native {
return {};
}
const uint32_t maxStorageBufferBindingSize =
const uint64_t maxStorageBufferBindingSize =
device->GetLimits().v1.maxStorageBufferBindingSize;
const uint32_t minStorageBufferOffsetAlignment =
device->GetLimits().v1.minStorageBufferOffsetAlignment;

View File

@ -22,6 +22,14 @@ constexpr uint32_t kRTSize = 4;
class DrawIndexedIndirectTest : public DawnTest {
protected:
wgpu::RequiredLimits GetRequiredLimits(const wgpu::SupportedLimits& supported) override {
// Force larger limits, that might reach into the upper 32 bits of the 64bit limit values,
// to help detect integer arithmetic bugs like overflows and truncations.
wgpu::RequiredLimits required = {};
required.limits = supported.limits;
return required;
}
void SetUp() override {
DawnTest::SetUp();