From 307919dba9f15ebe07ceb6328969e9f647171283 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Mon, 16 Nov 2020 15:21:07 +0000 Subject: [PATCH] reader/spirv: Replace std::make_unique -> create create() is currently just a simple forwarder to std::make_unique<>, but will be later replaced with a function that returns a raw pointer, and owned by the context. Bug: tint:322 Change-Id: I72558482c4b6aff7a655087ee010b3e16b006192 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/32860 Commit-Queue: Ben Clayton Reviewed-by: dan sinclair --- src/reader/spirv/function.cc | 283 +++++++++++++++----------------- src/reader/spirv/function.h | 8 + src/reader/spirv/parser_impl.cc | 125 +++++++------- src/reader/spirv/parser_impl.h | 7 + 4 files changed, 211 insertions(+), 212 deletions(-) diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc index dafb7ce381..3cf2389ac8 100644 --- a/src/reader/spirv/function.cc +++ b/src/reader/spirv/function.cc @@ -502,9 +502,8 @@ FunctionEmitter::StatementBlock::~StatementBlock() = default; void FunctionEmitter::PushNewStatementBlock(const Construct* construct, uint32_t end_id, CompletionAction action) { - statements_stack_.emplace_back( - StatementBlock{construct, end_id, action, - std::make_unique(), nullptr}); + statements_stack_.emplace_back(StatementBlock{ + construct, end_id, action, create(), nullptr}); } void FunctionEmitter::PushGuard(const std::string& guard_name, @@ -515,11 +514,11 @@ void FunctionEmitter::PushGuard(const std::string& guard_name, // if-selection with a then-clause ending at the same block // as the statement block at the top of the stack. const auto& top = statements_stack_.back(); - auto cond = std::make_unique(guard_name); - auto body = std::make_unique(); - auto* const guard_stmt = AddStatement(std::make_unique( - std::move(cond), std::move(body))) - ->AsIf(); + auto cond = create(guard_name); + auto body = create(); + auto* const guard_stmt = + AddStatement(create(std::move(cond), std::move(body))) + ->AsIf(); PushNewStatementBlock(top.construct_, end_id, [guard_stmt](StatementBlock* s) { guard_stmt->set_body(std::move(s->statements_)); @@ -530,10 +529,10 @@ void FunctionEmitter::PushTrueGuard(uint32_t end_id) { assert(!statements_stack_.empty()); const auto& top = statements_stack_.back(); auto cond = MakeTrue(); - auto body = std::make_unique(); - auto* const guard_stmt = AddStatement(std::make_unique( - std::move(cond), std::move(body))) - ->AsIf(); + auto body = create(); + auto* const guard_stmt = + AddStatement(create(std::move(cond), std::move(body))) + ->AsIf(); guard_stmt->set_condition(MakeTrue()); PushNewStatementBlock(top.construct_, end_id, [guard_stmt](StatementBlock* s) { @@ -649,13 +648,12 @@ bool FunctionEmitter::EmitFunctionDeclaration() { return false; } - auto ast_fn = - std::make_unique(name, std::move(ast_params), ret_ty, - std::make_unique()); + auto ast_fn = create(name, std::move(ast_params), ret_ty, + create()); if (ep_info_ != nullptr) { ast_fn->add_decoration( - std::make_unique(ep_info_->stage, Source{})); + create(ep_info_->stage, Source{})); } ast_module_.AddFunction(std::move(ast_fn)); @@ -1707,8 +1705,7 @@ bool FunctionEmitter::EmitFunctionVariables() { parser_impl_.MakeConstantExpression(inst.GetSingleWordInOperand(1)) .expr); } - auto var_decl_stmt = - std::make_unique(std::move(var)); + auto var_decl_stmt = create(std::move(var)); AddStatementForInstruction(std::move(var_decl_stmt), inst); // Save this as an already-named value. identifier_values_.insert(inst.result_id()); @@ -1723,7 +1720,7 @@ TypedExpression FunctionEmitter::MakeExpression(uint32_t id) { if (identifier_values_.count(id) || parser_impl_.IsScalarSpecConstant(id)) { return TypedExpression( parser_impl_.ConvertType(def_use_mgr_->GetDef(id)->type_id()), - std::make_unique(namer_.Name(id))); + create(namer_.Name(id))); } if (singly_used_values_.count(id)) { auto expr = std::move(singly_used_values_[id]); @@ -1742,9 +1739,9 @@ TypedExpression FunctionEmitter::MakeExpression(uint32_t id) { switch (inst->opcode()) { case SpvOpVariable: // This occurs for module-scope variables. - return TypedExpression(parser_impl_.ConvertType(inst->type_id()), - std::make_unique( - namer_.Name(inst->result_id()))); + return TypedExpression( + parser_impl_.ConvertType(inst->type_id()), + create(namer_.Name(inst->result_id()))); default: break; } @@ -1973,21 +1970,20 @@ bool FunctionEmitter::EmitIfStart(const BlockInfo& block_info) { const std::string guard_name = block_info.flow_guard_name; if (!guard_name.empty()) { // Declare the guard variable just before the "if", initialized to true. - auto guard_var = std::make_unique( + auto guard_var = create( guard_name, ast::StorageClass::kFunction, parser_impl_.BoolType()); guard_var->set_constructor(MakeTrue()); - auto guard_decl = - std::make_unique(std::move(guard_var)); + auto guard_decl = create(std::move(guard_var)); AddStatement(std::move(guard_decl)); } const auto condition_id = block_info.basic_block->terminator()->GetSingleWordInOperand(0); auto cond = MakeExpression(condition_id).expr; - auto body = std::make_unique(); - auto* const if_stmt = AddStatement(std::make_unique( - std::move(cond), std::move(body))) - ->AsIf(); + auto body = create(); + auto* const if_stmt = + AddStatement(create(std::move(cond), std::move(body))) + ->AsIf(); // Generate the code for the condition. @@ -2040,17 +2036,18 @@ bool FunctionEmitter::EmitIfStart(const BlockInfo& block_info) { auto push_else = [this, if_stmt, else_end, construct]() { // Push the else clause onto the stack first. - PushNewStatementBlock(construct, else_end, [if_stmt](StatementBlock* s) { - // Only set the else-clause if there are statements to fill it. - if (!s->statements_->empty()) { - // The "else" consists of the statement list from the top of statments - // stack, without an elseif condition. - ast::ElseStatementList else_stmts; - else_stmts.emplace_back(std::make_unique( - nullptr, std::move(s->statements_))); - if_stmt->set_else_statements(std::move(else_stmts)); - } - }); + PushNewStatementBlock( + construct, else_end, [this, if_stmt](StatementBlock* s) { + // Only set the else-clause if there are statements to fill it. + if (!s->statements_->empty()) { + // The "else" consists of the statement list from the top of + // statments stack, without an elseif condition. + ast::ElseStatementList else_stmts; + else_stmts.emplace_back( + create(nullptr, std::move(s->statements_))); + if_stmt->set_else_statements(std::move(else_stmts)); + } + }); }; if (GetBlockInfo(else_end)->pos < GetBlockInfo(then_end)->pos) { @@ -2099,7 +2096,7 @@ bool FunctionEmitter::EmitSwitchStart(const BlockInfo& block_info) { const auto* branch = block_info.basic_block->terminator(); auto* const switch_stmt = - AddStatement(std::make_unique())->AsSwitch(); + AddStatement(create())->AsSwitch(); const auto selector_id = branch->GetSingleWordInOperand(0); // Generate the code for the selector. auto selector = MakeExpression(selector_id); @@ -2111,7 +2108,7 @@ bool FunctionEmitter::EmitSwitchStart(const BlockInfo& block_info) { construct, construct->end_id, [switch_stmt](StatementBlock* s) { switch_stmt->set_body(std::move(*std::move(s->cases_))); }); - statements_stack_.back().cases_ = std::make_unique(); + statements_stack_.back().cases_ = create(); // Grab a pointer to the case list. It will get buried in the statement block // stack. auto* cases = statements_stack_.back().cases_.get(); @@ -2157,8 +2154,8 @@ bool FunctionEmitter::EmitSwitchStart(const BlockInfo& block_info) { for (size_t i = last_clause_index;; --i) { // Create the case clause. Temporarily put it in the wrong order // on the case statement list. - cases->emplace_back(std::make_unique( - std::make_unique())); + cases->emplace_back( + create(create())); auto* clause = cases->back().get(); // Create a list of integer literals for the selector values leading to @@ -2175,10 +2172,10 @@ bool FunctionEmitter::EmitSwitchStart(const BlockInfo& block_info) { const uint32_t value32 = uint32_t(value & 0xFFFFFFFF); if (selector.type->is_unsigned_scalar_or_vector()) { selectors.emplace_back( - std::make_unique(selector.type, value32)); + create(selector.type, value32)); } else { selectors.emplace_back( - std::make_unique(selector.type, value32)); + create(selector.type, value32)); } } clause->set_selectors(std::move(selectors)); @@ -2195,9 +2192,9 @@ bool FunctionEmitter::EmitSwitchStart(const BlockInfo& block_info) { if ((default_info == clause_heads[i]) && has_selectors && construct->ContainsPos(default_info->pos)) { // Generate a default clause with a just fallthrough. - auto stmts = std::make_unique(); - stmts->append(std::make_unique()); - auto case_stmt = std::make_unique(std::move(stmts)); + auto stmts = create(); + stmts->append(create()); + auto case_stmt = create(std::move(stmts)); cases->emplace_back(std::move(case_stmt)); } @@ -2214,10 +2211,10 @@ bool FunctionEmitter::EmitSwitchStart(const BlockInfo& block_info) { } bool FunctionEmitter::EmitLoopStart(const Construct* construct) { - auto* loop = AddStatement(std::make_unique( - std::make_unique(), - std::make_unique())) - ->AsLoop(); + auto* loop = + AddStatement(create(create(), + create())) + ->AsLoop(); PushNewStatementBlock( construct, construct->end_id, [loop](StatementBlock* s) { loop->set_body(std::move(s->statements_)); }); @@ -2244,18 +2241,17 @@ bool FunctionEmitter::EmitNormalTerminator(const BlockInfo& block_info) { const auto& terminator = *(block_info.basic_block->terminator()); switch (terminator.opcode()) { case SpvOpReturn: - AddStatement(std::make_unique()); + AddStatement(create()); return true; case SpvOpReturnValue: { auto value = MakeExpression(terminator.GetSingleWordInOperand(0)); - AddStatement( - std::make_unique(std::move(value.expr))); + AddStatement(create(std::move(value.expr))); } return true; case SpvOpKill: // For now, assume SPIR-V OpKill has same semantics as WGSL discard. // TODO(dneto): https://github.com/gpuweb/gpuweb/issues/676 - AddStatement(std::make_unique()); + AddStatement(create()); return true; case SpvOpUnreachable: // Translate as if it's a return. This avoids the problem where WGSL @@ -2263,10 +2259,10 @@ bool FunctionEmitter::EmitNormalTerminator(const BlockInfo& block_info) { { const auto* result_type = type_mgr_->GetType(function_.type_id()); if (result_type->AsVoid() != nullptr) { - AddStatement(std::make_unique()); + AddStatement(create()); } else { auto* ast_type = parser_impl_.ConvertType(function_.type_id()); - AddStatement(std::make_unique( + AddStatement(create( parser_impl_.MakeNullValue(ast_type))); } } @@ -2350,7 +2346,7 @@ std::unique_ptr FunctionEmitter::MakeBranchDetailed( break; case EdgeKind::kSwitchBreak: { if (forced) { - return std::make_unique(); + return create(); } // Unless forced, don't bother with a break at the end of a case/default // clause. @@ -2375,10 +2371,10 @@ std::unique_ptr FunctionEmitter::MakeBranchDetailed( } } // We need a break. - return std::make_unique(); + return create(); } case EdgeKind::kLoopBreak: - return std::make_unique(); + return create(); case EdgeKind::kLoopContinue: // An unconditional continue to the next block is redundant and ugly. // Skip it in that case. @@ -2386,7 +2382,7 @@ std::unique_ptr FunctionEmitter::MakeBranchDetailed( break; } // Otherwise, emit a regular continue statement. - return std::make_unique(); + return create(); case EdgeKind::kIfBreak: { const auto& flow_guard = GetBlockInfo(dest_info.header_for_merge)->flow_guard_name; @@ -2395,9 +2391,8 @@ std::unique_ptr FunctionEmitter::MakeBranchDetailed( *flow_guard_name_ptr = flow_guard; } // Signal an exit from the branch. - return std::make_unique( - std::make_unique(flow_guard), - MakeFalse()); + return create( + create(flow_guard), MakeFalse()); } // For an unconditional branch, the break out to an if-selection @@ -2405,7 +2400,7 @@ std::unique_ptr FunctionEmitter::MakeBranchDetailed( break; } case EdgeKind::kCaseFallThrough: - return std::make_unique(); + return create(); case EdgeKind::kForward: // Unconditional forward branch is implicit. break; @@ -2420,19 +2415,19 @@ std::unique_ptr FunctionEmitter::MakeSimpleIf( if ((then_stmt == nullptr) && (else_stmt == nullptr)) { return nullptr; } - auto if_stmt = std::make_unique( - std::move(condition), std::make_unique()); + auto if_stmt = create(std::move(condition), + create()); if (then_stmt != nullptr) { - auto stmts = std::make_unique(); + auto stmts = create(); stmts->append(std::move(then_stmt)); if_stmt->set_body(std::move(stmts)); } if (else_stmt != nullptr) { - auto stmts = std::make_unique(); + auto stmts = create(); stmts->append(std::move(else_stmt)); ast::ElseStatementList else_stmts; else_stmts.emplace_back( - std::make_unique(nullptr, std::move(stmts))); + create(nullptr, std::move(stmts))); if_stmt->set_else_statements(std::move(else_stmts)); } return if_stmt; @@ -2479,7 +2474,7 @@ bool FunctionEmitter::EmitConditionalCaseFallThrough( AddStatement( MakeSimpleIf(std::move(cond), std::move(other_branch), nullptr)); } - AddStatement(std::make_unique()); + AddStatement(create()); return success(); } @@ -2506,7 +2501,7 @@ bool FunctionEmitter::EmitStatementsInBasicBlock(const BlockInfo& block_info, assert(def_inst); auto* ast_type = RemapStorageClass(parser_impl_.ConvertType(def_inst->type_id()), id); - AddStatement(std::make_unique( + AddStatement(create( parser_impl_.MakeVariable(id, ast::StorageClass::kFunction, ast_type))); // Save this as an already-named value. identifier_values_.insert(id); @@ -2517,10 +2512,10 @@ bool FunctionEmitter::EmitStatementsInBasicBlock(const BlockInfo& block_info, assert(def_inst); const auto phi_var_name = GetDefInfo(id)->phi_var; assert(!phi_var_name.empty()); - auto var = std::make_unique( - phi_var_name, ast::StorageClass::kFunction, - parser_impl_.ConvertType(def_inst->type_id())); - AddStatement(std::make_unique(std::move(var))); + auto var = + create(phi_var_name, ast::StorageClass::kFunction, + parser_impl_.ConvertType(def_inst->type_id())); + AddStatement(create(std::move(var))); } // Emit regular statements. @@ -2550,9 +2545,8 @@ bool FunctionEmitter::EmitStatementsInBasicBlock(const BlockInfo& block_info, for (auto assignment : block_info.phi_assignments) { const auto var_name = GetDefInfo(assignment.phi_id)->phi_var; auto expr = MakeExpression(assignment.value); - AddStatement(std::make_unique( - std::make_unique(var_name), - std::move(expr.expr))); + AddStatement(create( + create(var_name), std::move(expr.expr))); } } @@ -2574,7 +2568,7 @@ bool FunctionEmitter::EmitConstDefinition( ast_const->set_constructor(std::move(ast_expr.expr)); ast_const->set_is_const(true); AddStatementForInstruction( - std::make_unique(std::move(ast_const)), inst); + create(std::move(ast_const)), inst); // Save this as an already-named value. identifier_values_.insert(inst.result_id()); return success(); @@ -2588,8 +2582,8 @@ bool FunctionEmitter::EmitConstDefOrWriteToHoistedVar( if (def_info && def_info->requires_hoisted_def) { // Emit an assignment of the expression to the hoisted variable. AddStatementForInstruction( - std::make_unique( - std::make_unique(namer_.Name(result_id)), + create( + create(namer_.Name(result_id)), std::move(ast_expr.expr)), inst); return true; @@ -2654,7 +2648,7 @@ bool FunctionEmitter::EmitStatement(const spvtools::opt::Instruction& inst) { // TODO(dneto): Order of evaluation? auto lhs = MakeExpression(ptr_id); auto rhs = MakeExpression(value_id); - AddStatementForInstruction(std::make_unique( + AddStatementForInstruction(create( std::move(lhs.expr), std::move(rhs.expr)), inst); return success(); @@ -2678,9 +2672,9 @@ bool FunctionEmitter::EmitStatement(const spvtools::opt::Instruction& inst) { } case SpvOpPhi: { // Emit a read from the associated state variable. - auto expr = TypedExpression( - parser_impl_.ConvertType(inst.type_id()), - std::make_unique(def_info->phi_var)); + auto expr = + TypedExpression(parser_impl_.ConvertType(inst.type_id()), + create(def_info->phi_var)); return EmitConstDefOrWriteToHoistedVar(inst, std::move(expr)); } case SpvOpFunctionCall: @@ -2714,7 +2708,7 @@ TypedExpression FunctionEmitter::MaybeEmitCombinatorialValue( if (binary_op != ast::BinaryOp::kNone) { auto arg0 = MakeOperand(inst, 0); auto arg1 = MakeOperand(inst, 1); - auto binary_expr = std::make_unique( + auto binary_expr = create( binary_op, std::move(arg0.expr), std::move(arg1.expr)); TypedExpression result(ast_type, std::move(binary_expr)); return parser_impl_.RectifyForcedResultType(std::move(result), opcode, @@ -2724,8 +2718,8 @@ TypedExpression FunctionEmitter::MaybeEmitCombinatorialValue( auto unary_op = ast::UnaryOp::kNegation; if (GetUnaryOp(opcode, &unary_op)) { auto arg0 = MakeOperand(inst, 0); - auto unary_expr = std::make_unique( - unary_op, std::move(arg0.expr)); + auto unary_expr = + create(unary_op, std::move(arg0.expr)); TypedExpression result(ast_type, std::move(unary_expr)); return parser_impl_.RectifyForcedResultType(std::move(result), opcode, arg0.type); @@ -2735,10 +2729,9 @@ TypedExpression FunctionEmitter::MaybeEmitCombinatorialValue( if (unary_builtin_name != nullptr) { ast::ExpressionList params; params.emplace_back(MakeOperand(inst, 0).expr); - return {ast_type, - std::make_unique( - std::make_unique(unary_builtin_name), - std::move(params))}; + return {ast_type, create( + create(unary_builtin_name), + std::move(params))}; } const auto intrinsic = GetIntrinsic(opcode); @@ -2751,7 +2744,7 @@ TypedExpression FunctionEmitter::MaybeEmitCombinatorialValue( } if (opcode == SpvOpBitcast) { - return {ast_type, std::make_unique( + return {ast_type, create( ast_type, MakeOperand(inst, 0).expr)}; } @@ -2759,10 +2752,10 @@ TypedExpression FunctionEmitter::MaybeEmitCombinatorialValue( if (negated_op != ast::BinaryOp::kNone) { auto arg0 = MakeOperand(inst, 0); auto arg1 = MakeOperand(inst, 1); - auto binary_expr = std::make_unique( + auto binary_expr = create( negated_op, std::move(arg0.expr), std::move(arg1.expr)); - auto negated_expr = std::make_unique( - ast::UnaryOp::kNot, std::move(binary_expr)); + auto negated_expr = create(ast::UnaryOp::kNot, + std::move(binary_expr)); return {ast_type, std::move(negated_expr)}; } @@ -2780,7 +2773,7 @@ TypedExpression FunctionEmitter::MaybeEmitCombinatorialValue( for (uint32_t iarg = 0; iarg < inst.NumInOperands(); ++iarg) { operands.emplace_back(MakeOperand(inst, iarg).expr); } - return {ast_type, std::make_unique( + return {ast_type, create( ast_type, std::move(operands))}; } @@ -2838,15 +2831,14 @@ TypedExpression FunctionEmitter::EmitGlslStd450ExtInst( return {}; } - auto func = std::make_unique(name); + auto func = create(name); ast::ExpressionList operands; // All parameters to GLSL.std.450 extended instructions are IDs. for (uint32_t iarg = 2; iarg < inst.NumInOperands(); ++iarg) { operands.emplace_back(MakeOperand(inst, iarg).expr); } auto* ast_type = parser_impl_.ConvertType(inst.type_id()); - auto call = std::make_unique(std::move(func), - std::move(operands)); + auto call = create(std::move(func), std::move(operands)); return {ast_type, std::move(call)}; } @@ -2913,7 +2905,7 @@ TypedExpression FunctionEmitter::MakeAccessChain( // Replace the gl_PerVertex reference with the gl_Position reference ptr_ty_id = builtin_position_info.member_pointer_type_id; current_expr.expr = - std::make_unique(namer_.Name(base_id)); + create(namer_.Name(base_id)); current_expr.type = parser_impl_.ConvertType(ptr_ty_id); } } @@ -2963,13 +2955,13 @@ TypedExpression FunctionEmitter::MakeAccessChain( << " is too big. Max handled index is " << ((sizeof(swizzles) / sizeof(swizzles[0])) - 1); } - auto letter_index = std::make_unique( - swizzles[index_const_val]); - next_expr = std::make_unique( + auto letter_index = + create(swizzles[index_const_val]); + next_expr = create( std::move(current_expr.expr), std::move(letter_index)); } else { // Non-constant index. Use array syntax - next_expr = std::make_unique( + next_expr = create( std::move(current_expr.expr), std::move(MakeOperand(inst, index).expr)); } @@ -2978,20 +2970,20 @@ TypedExpression FunctionEmitter::MakeAccessChain( break; case SpvOpTypeMatrix: // Use array syntax. - next_expr = std::make_unique( + next_expr = create( std::move(current_expr.expr), std::move(MakeOperand(inst, index).expr)); // All matrix components are the same type. pointee_type_id = pointee_type_inst->GetSingleWordInOperand(0); break; case SpvOpTypeArray: - next_expr = std::make_unique( + next_expr = create( std::move(current_expr.expr), std::move(MakeOperand(inst, index).expr)); pointee_type_id = pointee_type_inst->GetSingleWordInOperand(0); break; case SpvOpTypeRuntimeArray: - next_expr = std::make_unique( + next_expr = create( std::move(current_expr.expr), std::move(MakeOperand(inst, index).expr)); pointee_type_id = pointee_type_inst->GetSingleWordInOperand(0); @@ -3011,10 +3003,10 @@ TypedExpression FunctionEmitter::MakeAccessChain( << pointee_type_id << " having " << num_members << " members"; return {}; } - auto member_access = std::make_unique( + auto member_access = create( namer_.GetMemberName(pointee_type_id, uint32_t(index_const_val))); - next_expr = std::make_unique( + next_expr = create( std::move(current_expr.expr), std::move(member_access)); pointee_type_id = pointee_type_inst->GetSingleWordInOperand( static_cast(index_const_val)); @@ -3047,10 +3039,10 @@ TypedExpression FunctionEmitter::MakeCompositeExtract( // expressions. TypedExpression current_expr(MakeOperand(inst, 0)); - auto make_index = [](uint32_t literal) { + auto make_index = [this](uint32_t literal) { ast::type::U32Type u32; - return std::make_unique( - std::make_unique(&u32, literal)); + return create( + create(&u32, literal)); }; static const char* swizzles[] = {"x", "y", "z", "w"}; @@ -3088,8 +3080,8 @@ TypedExpression FunctionEmitter::MakeCompositeExtract( << ((sizeof(swizzles) / sizeof(swizzles[0])) - 1); } auto letter_index = - std::make_unique(swizzles[index_val]); - next_expr = std::make_unique( + create(swizzles[index_val]); + next_expr = create( std::move(current_expr.expr), std::move(letter_index)); // All vector components are the same type. current_type_id = current_type_inst->GetSingleWordInOperand(0); @@ -3110,7 +3102,7 @@ TypedExpression FunctionEmitter::MakeCompositeExtract( << ((sizeof(swizzles) / sizeof(swizzles[0])) - 1); } // Use array syntax. - next_expr = std::make_unique( + next_expr = create( std::move(current_expr.expr), make_index(index_val)); // All matrix components are the same type. current_type_id = current_type_inst->GetSingleWordInOperand(0); @@ -3120,7 +3112,7 @@ TypedExpression FunctionEmitter::MakeCompositeExtract( // The array size could be a spec constant, and so it's not always // statically checkable. Instead, rely on a runtime index clamp // or runtime check to keep this safe. - next_expr = std::make_unique( + next_expr = create( std::move(current_expr.expr), make_index(index_val)); current_type_id = current_type_inst->GetSingleWordInOperand(0); break; @@ -3135,10 +3127,10 @@ TypedExpression FunctionEmitter::MakeCompositeExtract( << current_type_id << " having " << num_members << " members"; return {}; } - auto member_access = std::make_unique( + auto member_access = create( namer_.GetMemberName(current_type_id, uint32_t(index_val))); - next_expr = std::make_unique( + next_expr = create( std::move(current_expr.expr), std::move(member_access)); current_type_id = current_type_inst->GetSingleWordInOperand(index_val); break; @@ -3155,14 +3147,14 @@ TypedExpression FunctionEmitter::MakeCompositeExtract( } std::unique_ptr FunctionEmitter::MakeTrue() const { - return std::make_unique( - std::make_unique(parser_impl_.BoolType(), true)); + return create( + create(parser_impl_.BoolType(), true)); } std::unique_ptr FunctionEmitter::MakeFalse() const { ast::type::BoolType bool_type; - return std::make_unique( - std::make_unique(parser_impl_.BoolType(), false)); + return create( + create(parser_impl_.BoolType(), false)); } TypedExpression FunctionEmitter::MakeVectorShuffle( @@ -3188,15 +3180,15 @@ TypedExpression FunctionEmitter::MakeVectorShuffle( const auto index = inst.GetSingleWordInOperand(i); if (index < vec0_len) { assert(index < sizeof(swizzles) / sizeof(swizzles[0])); - values.emplace_back(std::make_unique( + values.emplace_back(create( MakeExpression(vec0_id).expr, - std::make_unique(swizzles[index]))); + create(swizzles[index]))); } else if (index < vec0_len + vec1_len) { const auto sub_index = index - vec0_len; assert(sub_index < sizeof(swizzles) / sizeof(swizzles[0])); - values.emplace_back(std::make_unique( + values.emplace_back(create( MakeExpression(vec1_id).expr, - std::make_unique(swizzles[sub_index]))); + create(swizzles[sub_index]))); } else if (index == 0xFFFFFFFF) { // By rule, this maps to OpUndef. Instead, make it zero. values.emplace_back(parser_impl_.MakeNullValue(result_type->type())); @@ -3206,7 +3198,7 @@ TypedExpression FunctionEmitter::MakeVectorShuffle( return {}; } } - return {result_type, std::make_unique( + return {result_type, create( result_type, std::move(values))}; } @@ -3497,28 +3489,27 @@ TypedExpression FunctionEmitter::MakeNumericConversion( ast::ExpressionList params; params.push_back(std::move(arg_expr.expr)); - TypedExpression result(expr_type, - std::make_unique( - expr_type, std::move(params))); + TypedExpression result(expr_type, create( + expr_type, std::move(params))); if (requested_type == expr_type) { return result; } - return {requested_type, std::make_unique( + return {requested_type, create( requested_type, std::move(result.expr))}; } bool FunctionEmitter::EmitFunctionCall(const spvtools::opt::Instruction& inst) { // We ignore function attributes such as Inline, DontInline, Pure, Const. - auto function = std::make_unique( + auto function = create( namer_.Name(inst.GetSingleWordInOperand(0))); ast::ExpressionList params; for (uint32_t iarg = 1; iarg < inst.NumInOperands(); ++iarg) { params.emplace_back(MakeOperand(inst, iarg).expr); } - auto call_expr = std::make_unique(std::move(function), - std::move(params)); + auto call_expr = + create(std::move(function), std::move(params)); auto* result_type = parser_impl_.ConvertType(inst.type_id()); if (!result_type) { return Fail() << "internal error: no mapped type result of call: " @@ -3528,8 +3519,7 @@ bool FunctionEmitter::EmitFunctionCall(const spvtools::opt::Instruction& inst) { if (result_type->IsVoid()) { return nullptr != AddStatementForInstruction( - std::make_unique(std::move(call_expr)), - inst); + create(std::move(call_expr)), inst); } return EmitConstDefOrWriteToHoistedVar(inst, @@ -3541,15 +3531,15 @@ TypedExpression FunctionEmitter::MakeIntrinsicCall( const auto intrinsic = GetIntrinsic(inst.opcode()); std::ostringstream ss; ss << intrinsic; - auto ident = std::make_unique(ss.str()); + auto ident = create(ss.str()); ident->set_intrinsic(intrinsic); ast::ExpressionList params; for (uint32_t iarg = 0; iarg < inst.NumInOperands(); ++iarg) { params.emplace_back(MakeOperand(inst, iarg).expr); } - auto call_expr = std::make_unique(std::move(ident), - std::move(params)); + auto call_expr = + create(std::move(ident), std::move(params)); auto* result_type = parser_impl_.ConvertType(inst.type_id()); if (!result_type) { Fail() << "internal error: no mapped type result of call: " @@ -3578,10 +3568,9 @@ TypedExpression FunctionEmitter::MakeSimpleSelect( params.push_back(std::move(operand2.expr)); // The condition goes last. params.push_back(std::move(condition.expr)); - return {operand1.type, - std::make_unique( - std::make_unique("select"), - std::move(params))}; + return {operand1.type, create( + create("select"), + std::move(params))}; } return {}; } diff --git a/src/reader/spirv/function.h b/src/reader/spirv/function.h index c9df72aaa1..19323888e8 100644 --- a/src/reader/spirv/function.h +++ b/src/reader/spirv/function.h @@ -21,6 +21,7 @@ #include #include #include +#include #include #include "source/opt/basic_block.h" @@ -799,6 +800,13 @@ class FunctionEmitter { /// @returns a boolean false expression. std::unique_ptr MakeFalse() const; + /// @return a `std::unique_ptr` to a new `T` constructed with `args` + /// @param args the arguments to forward to the constructor for `T` + template + std::unique_ptr create(ARGS&&... args) const { + return std::make_unique(std::forward(args)...); + } + ParserImpl& parser_impl_; ast::Module& ast_module_; spvtools::opt::IRContext& ir_context_; diff --git a/src/reader/spirv/parser_impl.cc b/src/reader/spirv/parser_impl.cc index c36ec78834..63cc7c7834 100644 --- a/src/reader/spirv/parser_impl.cc +++ b/src/reader/spirv/parser_impl.cc @@ -392,8 +392,7 @@ ParserImpl::ConvertMemberDecoration(uint32_t struct_type_id, << ShowType(struct_type_id); return nullptr; } - return std::make_unique(decoration[1], - Source{}); + return create(decoration[1], Source{}); case SpvDecorationNonReadable: // WGSL doesn't have a member decoration for this. Silently drop it. return nullptr; @@ -782,8 +781,7 @@ bool ParserImpl::ApplyArrayDecorations( << ": multiple ArrayStride decorations"; } ast::ArrayDecorationList decos; - decos.push_back( - std::make_unique(stride, Source{})); + decos.push_back(create(stride, Source{})); ast_type->set_decorations(std::move(decos)); } else { return Fail() << "invalid array type ID " << type_id @@ -806,10 +804,10 @@ ast::type::Type* ParserImpl::ConvertType( const auto decoration = struct_decorations[0][0]; if (decoration == SpvDecorationBlock) { ast_struct_decorations.push_back( - std::make_unique(Source{})); + create(Source{})); } else if (decoration == SpvDecorationBufferBlock) { ast_struct_decorations.push_back( - std::make_unique(Source{})); + create(Source{})); remap_buffer_block_type_.insert(type_id); } else { Fail() << "struct with ID " << type_id @@ -879,14 +877,14 @@ ast::type::Type* ParserImpl::ConvertType( ++num_non_writable_members; } const auto member_name = namer_.GetMemberName(type_id, member_index); - auto ast_struct_member = std::make_unique( + auto ast_struct_member = create( member_name, ast_member_ty, std::move(ast_member_decorations)); ast_members.push_back(std::move(ast_struct_member)); } // Now make the struct. - auto ast_struct = std::make_unique( - std::move(ast_struct_decorations), std::move(ast_members)); + auto ast_struct = create(std::move(ast_struct_decorations), + std::move(ast_members)); namer_.SuggestSanitizedName(type_id, "S"); auto ast_struct_type = std::make_unique( @@ -971,8 +969,8 @@ bool ParserImpl::EmitScalarSpecConstants() { case SpvOpSpecConstantTrue: case SpvOpSpecConstantFalse: { ast_type = ConvertType(inst.type_id()); - ast_expr = std::make_unique( - std::make_unique( + ast_expr = + create(create( ast_type, inst.opcode() == SpvOpSpecConstantTrue)); break; } @@ -980,19 +978,19 @@ bool ParserImpl::EmitScalarSpecConstants() { ast_type = ConvertType(inst.type_id()); const uint32_t literal_value = inst.GetSingleWordInOperand(0); if (ast_type->IsI32()) { - ast_expr = std::make_unique( - std::make_unique( + ast_expr = + create(create( ast_type, static_cast(literal_value))); } else if (ast_type->IsU32()) { - ast_expr = std::make_unique( - std::make_unique( + ast_expr = + create(create( ast_type, static_cast(literal_value))); } else if (ast_type->IsF32()) { float float_value; // Copy the bits so we can read them as a float. std::memcpy(&float_value, &literal_value, sizeof(float_value)); - ast_expr = std::make_unique( - std::make_unique(ast_type, float_value)); + ast_expr = create( + create(ast_type, float_value)); } else { return Fail() << " invalid result type for OpSpecConstant " << inst.PrettyPrint(); @@ -1008,8 +1006,7 @@ bool ParserImpl::EmitScalarSpecConstants() { ast::VariableDecorationList spec_id_decos; for (const auto& deco : GetDecorationsFor(inst.result_id())) { if ((deco.size() == 2) && (deco[0] == SpvDecorationSpecId)) { - auto cid = - std::make_unique(deco[1], Source{}); + auto cid = create(deco[1], Source{}); spec_id_decos.push_back(std::move(cid)); break; } @@ -1020,8 +1017,7 @@ bool ParserImpl::EmitScalarSpecConstants() { ast_var->set_constructor(std::move(ast_expr)); ast_module_.AddGlobalVariable(std::move(ast_var)); } else { - auto ast_deco_var = - std::make_unique(std::move(ast_var)); + auto ast_deco_var = create(std::move(ast_var)); ast_deco_var->set_is_const(true); ast_deco_var->set_constructor(std::move(ast_expr)); ast_deco_var->set_decorations(std::move(spec_id_decos)); @@ -1140,13 +1136,13 @@ bool ParserImpl::EmitModuleScopeVariables() { // Make sure the variable has a name. namer_.SuggestSanitizedName(builtin_position_.per_vertex_var_id, "gl_Position"); - auto var = std::make_unique(MakeVariable( + auto var = create(MakeVariable( builtin_position_.per_vertex_var_id, enum_converter_.ToStorageClass(builtin_position_.storage_class), ConvertType(builtin_position_.member_type_id))); ast::VariableDecorationList decos; - decos.push_back(std::make_unique( - ast::Builtin::kPosition, Source{})); + decos.push_back( + create(ast::Builtin::kPosition, Source{})); var->set_decorations(std::move(decos)); ast_module_.AddGlobalVariable(std::move(var)); @@ -1171,7 +1167,7 @@ std::unique_ptr ParserImpl::MakeVariable(uint32_t id, std::make_unique(access, type)); } - auto ast_var = std::make_unique(namer_.Name(id), sc, type); + auto ast_var = create(namer_.Name(id), sc, type); ast::VariableDecorationList ast_decorations; for (auto& deco : GetDecorationsFor(id)) { @@ -1191,7 +1187,7 @@ std::unique_ptr ParserImpl::MakeVariable(uint32_t id, return nullptr; } ast_decorations.emplace_back( - std::make_unique(ast_builtin, Source{})); + create(ast_builtin, Source{})); } if (deco[0] == SpvDecorationLocation) { if (deco.size() != 2) { @@ -1200,7 +1196,7 @@ std::unique_ptr ParserImpl::MakeVariable(uint32_t id, return nullptr; } ast_decorations.emplace_back( - std::make_unique(deco[1], Source{})); + create(deco[1], Source{})); } if (deco[0] == SpvDecorationDescriptorSet) { if (deco.size() == 1) { @@ -1209,7 +1205,7 @@ std::unique_ptr ParserImpl::MakeVariable(uint32_t id, return nullptr; } ast_decorations.emplace_back( - std::make_unique(deco[1], Source{})); + create(deco[1], Source{})); } if (deco[0] == SpvDecorationBinding) { if (deco.size() == 1) { @@ -1218,12 +1214,11 @@ std::unique_ptr ParserImpl::MakeVariable(uint32_t id, return nullptr; } ast_decorations.emplace_back( - std::make_unique(deco[1], Source{})); + create(deco[1], Source{})); } } if (!ast_decorations.empty()) { - auto decorated_var = - std::make_unique(std::move(ast_var)); + auto decorated_var = create(std::move(ast_var)); decorated_var->set_decorations(std::move(ast_decorations)); ast_var = std::move(decorated_var); } @@ -1263,26 +1258,26 @@ TypedExpression ParserImpl::MakeConstantExpression(uint32_t id) { // Currently "null" is missing from the WGSL parser. // See https://bugs.chromium.org/p/tint/issues/detail?id=34 if (ast_type->IsU32()) { - return {ast_type, std::make_unique( - std::make_unique( - ast_type, spirv_const->GetU32()))}; + return {ast_type, + create( + create(ast_type, spirv_const->GetU32()))}; } if (ast_type->IsI32()) { - return {ast_type, std::make_unique( - std::make_unique( - ast_type, spirv_const->GetS32()))}; + return {ast_type, + create( + create(ast_type, spirv_const->GetS32()))}; } if (ast_type->IsF32()) { - return {ast_type, std::make_unique( - std::make_unique( - ast_type, spirv_const->GetFloat()))}; + return {ast_type, + create( + create(ast_type, spirv_const->GetFloat()))}; } if (ast_type->IsBool()) { const bool value = spirv_const->AsNullConstant() ? false : spirv_const->AsBoolConstant()->value(); - return {ast_type, std::make_unique( - std::make_unique(ast_type, value))}; + return {ast_type, create( + create(ast_type, value))}; } auto* spirv_composite_const = spirv_const->AsCompositeConstant(); if (spirv_composite_const != nullptr) { @@ -1308,8 +1303,8 @@ TypedExpression ParserImpl::MakeConstantExpression(uint32_t id) { ast_components.emplace_back(std::move(ast_component.expr)); } return {original_ast_type, - std::make_unique( - original_ast_type, std::move(ast_components))}; + create(original_ast_type, + std::move(ast_components))}; } auto* spirv_null_const = spirv_const->AsNullConstant(); if (spirv_null_const != nullptr) { @@ -1336,20 +1331,20 @@ std::unique_ptr ParserImpl::MakeNullValue( type = type->UnwrapIfNeeded(); if (type->IsBool()) { - return std::make_unique( - std::make_unique(type, false)); + return create( + create(type, false)); } if (type->IsU32()) { - return std::make_unique( - std::make_unique(type, 0u)); + return create( + create(type, 0u)); } if (type->IsI32()) { - return std::make_unique( - std::make_unique(type, 0)); + return create( + create(type, 0)); } if (type->IsF32()) { - return std::make_unique( - std::make_unique(type, 0.0f)); + return create( + create(type, 0.0f)); } if (type->IsVector()) { const auto* vec_ty = type->AsVector(); @@ -1357,8 +1352,8 @@ std::unique_ptr ParserImpl::MakeNullValue( for (size_t i = 0; i < vec_ty->size(); ++i) { ast_components.emplace_back(MakeNullValue(vec_ty->type())); } - return std::make_unique( - type, std::move(ast_components)); + return create(type, + std::move(ast_components)); } if (type->IsMatrix()) { const auto* mat_ty = type->AsMatrix(); @@ -1370,8 +1365,8 @@ std::unique_ptr ParserImpl::MakeNullValue( for (size_t i = 0; i < mat_ty->columns(); ++i) { ast_components.emplace_back(MakeNullValue(column_ty)); } - return std::make_unique( - type, std::move(ast_components)); + return create(type, + std::move(ast_components)); } if (type->IsArray()) { auto* arr_ty = type->AsArray(); @@ -1379,8 +1374,8 @@ std::unique_ptr ParserImpl::MakeNullValue( for (size_t i = 0; i < arr_ty->size(); ++i) { ast_components.emplace_back(MakeNullValue(arr_ty->type())); } - return std::make_unique( - original_type, std::move(ast_components)); + return create(original_type, + std::move(ast_components)); } if (type->IsStruct()) { auto* struct_ty = type->AsStruct(); @@ -1388,8 +1383,8 @@ std::unique_ptr ParserImpl::MakeNullValue( for (auto& member : struct_ty->impl()->members()) { ast_components.emplace_back(MakeNullValue(member->type())); } - return std::make_unique( - original_type, std::move(ast_components)); + return create(original_type, + std::move(ast_components)); } Fail() << "can't make null value for type: " << type->type_name(); return nullptr; @@ -1416,15 +1411,15 @@ TypedExpression ParserImpl::RectifyOperandSignedness(SpvOp op, auto* unsigned_ty = unsigned_type_for_[type]; if (unsigned_ty != nullptr) { // Conversion is required. - return {unsigned_ty, std::make_unique( + return {unsigned_ty, create( unsigned_ty, std::move(expr.expr))}; } } else if (requires_signed) { auto* signed_ty = signed_type_for_[type]; if (signed_ty != nullptr) { // Conversion is required. - return {signed_ty, std::make_unique( - signed_ty, std::move(expr.expr))}; + return {signed_ty, + create(signed_ty, std::move(expr.expr))}; } } // We should not reach here. @@ -1487,8 +1482,8 @@ TypedExpression ParserImpl::RectifyForcedResultType( if ((forced_result_ty == nullptr) || (forced_result_ty == expr.type)) { return expr; } - return {expr.type, std::make_unique( - expr.type, std::move(expr.expr))}; + return {expr.type, + create(expr.type, std::move(expr.expr))}; } bool ParserImpl::EmitFunctions() { diff --git a/src/reader/spirv/parser_impl.h b/src/reader/spirv/parser_impl.h index 70373972aa..5eaaf08e35 100644 --- a/src/reader/spirv/parser_impl.h +++ b/src/reader/spirv/parser_impl.h @@ -432,6 +432,13 @@ class ParserImpl : Reader { bool ApplyArrayDecorations(const spvtools::opt::analysis::Type* spv_type, ast::type::ArrayType* ast_type); + /// @return a `std::unique_ptr` to a new `T` constructed with `args` + /// @param args the arguments to forward to the constructor for `T` + template + std::unique_ptr create(ARGS&&... args) const { + return std::make_unique(std::forward(args)...); + } + // The SPIR-V binary we're parsing std::vector spv_binary_;