Skip to content

Commit

Permalink
Drop unnecessary arg-ids in llvm::formatv (iree-org#19426)
Browse files Browse the repository at this point in the history
`llvm::formatv` supports (approximately?) the same format syntax as
`std::format` described
[here](https://en.cppreference.com/w/cpp/utility/format/format). The
general pattern is `{arg-id}` or `{arg-id:format-spec}` where `arg-id`
is optional and may be omitted to default to consuming the args in the
order in which they are provided. So the only reason to ever specify
`arg-id` is to consume the same arg repeatedly or in a different order.

It turns out that 100% of the call sites were consuming args in the
order in which they were specified, so the arg-ids were unnecessary.

Signed-off-by: Benoit Jacob <jacob.benoit.1@gmail.com>
  • Loading branch information
bjacob authored Dec 9, 2024
1 parent 1b0cb44 commit b2c5f3b
Show file tree
Hide file tree
Showing 23 changed files with 69 additions and 74 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -220,12 +220,12 @@ class WindowsLinkerTool : public LinkerTool {
return std::nullopt;
}
flags.push_back(
llvm::formatv("/libpath:\"{0}\\lib\\{1}\"", "%VCToolsInstallDir%", arch)
llvm::formatv("/libpath:\"{}\\lib\\{}\"", "%VCToolsInstallDir%", arch)
.str());
flags.push_back(llvm::formatv("/libpath:\"{0}\\Lib\\{1}\\ucrt\\{2}\"",
flags.push_back(llvm::formatv("/libpath:\"{}\\Lib\\{}\\ucrt\\{}\"",
"%UniversalCRTSdkDir%", "%UCRTVersion%", arch)
.str());
flags.push_back(llvm::formatv("/libpath:\"{0}\\Lib\\{1}\\um\\{2}\"",
flags.push_back(llvm::formatv("/libpath:\"{}\\Lib\\{}\\um\\{}\"",
"%UniversalCRTSdkDir%", "%UCRTVersion%", arch)
.str());

Expand Down
2 changes: 1 addition & 1 deletion compiler/plugins/target/ROCM/ROCMTarget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ class ROCMTargetBackend final : public TargetBackend {
return variantOp.emitError()
<< "found an unresolved external function '" << func.getName()
<< "' in the final bitcode. A remaining live user is\n"
<< llvm::formatv("{0}", *liveUser);
<< llvm::formatv("{}", *liveUser);
}
}
return success();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ bool GpuHello::runOnModule(llvm::Module &module) {
if (!debugLocation) {
sourceInfo = function.getName().str();
} else {
sourceInfo = llvm::formatv("{0}\t{1}:{2}:{3}", function.getName(),
sourceInfo = llvm::formatv("{}\t{}:{}:{}", function.getName(),
debugLocation->getFilename(),
debugLocation->getLine(),
debugLocation->getColumn())
Expand Down
2 changes: 1 addition & 1 deletion compiler/plugins/target/WebGPUSPIRV/WebGPUSPIRVTarget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ class WebGPUSPIRVTargetBackend : public TargetBackend {
auto entryPointFunc = dyn_cast<spirv::FuncOp>(
SymbolTable::lookupSymbolIn(spvModuleOp, exportOp.getSymName()));

std::string symbolName = llvm::formatv("d{0}", exportOp.getOrdinal());
std::string symbolName = llvm::formatv("d{}", exportOp.getOrdinal());
mlir::StringAttr nameAttr =
mlir::StringAttr::get(variantOp->getContext(), symbolName);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ struct ConvertHalInterfaceBindingSubspan final
if (!newResultTy)
return rewriter.notifyMatchFailure(
op->getLoc(),
llvm::formatv("failed to legalize memref type: {0}", op.getType()));
llvm::formatv("failed to legalize memref type: {}", op.getType()));

auto newOp =
rewriter.replaceOpWithNewOp<IREE::HAL::InterfaceBindingSubspanOp>(
Expand All @@ -111,7 +111,7 @@ struct ConvertMemRefAlloc final : OpConversionPattern<memref::AllocOp> {
if (!newTy)
return rewriter.notifyMatchFailure(
op->getLoc(),
llvm::formatv("failed to convert memref type: {0}", op.getType()));
llvm::formatv("failed to convert memref type: {}", op.getType()));

rewriter.replaceOpWithNewOp<memref::AllocOp>(
op, newTy, adaptor.getDynamicSizes(), adaptor.getSymbolOperands(),
Expand Down Expand Up @@ -196,7 +196,7 @@ struct ConvertMemRefLoad final : OpConversionPattern<memref::LoadOp> {
Type newResTy = getTypeConverter()->convertType(op.getType());
if (!newResTy)
return rewriter.notifyMatchFailure(
op->getLoc(), llvm::formatv("failed to convert memref type: {0}",
op->getLoc(), llvm::formatv("failed to convert memref type: {}",
op.getMemRefType()));

rewriter.replaceOpWithNewOp<memref::LoadOp>(
Expand All @@ -215,7 +215,7 @@ struct ConvertMemRefStore final : OpConversionPattern<memref::StoreOp> {
Type newTy = getTypeConverter()->convertType(op.getMemRefType());
if (!newTy)
return rewriter.notifyMatchFailure(
op->getLoc(), llvm::formatv("failed to convert memref type: {0}",
op->getLoc(), llvm::formatv("failed to convert memref type: {}",
op.getMemRefType()));

rewriter.replaceOpWithNewOp<memref::StoreOp>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ struct ConvertHalInterfaceBindingSubspan final
if (!newResultType) {
return rewriter.notifyMatchFailure(
op->getLoc(),
llvm::formatv("failed to legalize memref type: {0}", op.getType()));
llvm::formatv("failed to legalize memref type: {}", op.getType()));
}
Location loc = op.getLoc();
OpFoldResult zero = rewriter.getIndexAttr(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ getUKernelBitcode(OpBuilder &builder,
return {};
}
StringRef gpuArch = gpuTarget.getArch();
std::string bitcodeFilename =
llvm::formatv("{0}.{1}.bc", ukernelName, gpuArch);
std::string bitcodeFilename = llvm::formatv("{}.{}.bc", ukernelName, gpuArch);

// Early-return if the source executable.objects already contain an object
// with the expected file name. This happens with user-provided bitcode in the
Expand Down Expand Up @@ -121,7 +120,7 @@ getFnNameAndDefAttrs(const char *name, std::string &suffix,
IREE::HAL::ExecutableTargetAttr targetAttr) {
FnNameAndDefAttrs result;
if (isROCMBackend(targetAttr)) {
result.name = llvm::formatv("iree_uk_amdgpu_{0}_{1}", name, suffix);
result.name = llvm::formatv("iree_uk_amdgpu_{}_{}", name, suffix);
result.defAttrs.emplace_back(rewriter.getStringAttr("vm.import.module"),
rewriter.getStringAttr("rocm"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ static FailureOr<func::CallOp> lowerUKernelGenericToFunctionCall(
if (failed(getCallOpType(rewriter.getContext(), microKernelOpOperandType,
stridedOuterDimsAttr, callArgumentTypes))) {
return rewriter.notifyMatchFailure(
op, llvm::formatv("failed to lower operand type {0}",
op, llvm::formatv("failed to lower operand type {}",
microKernelOpOperandType));
}
}
Expand All @@ -156,7 +156,7 @@ static FailureOr<func::CallOp> lowerUKernelGenericToFunctionCall(
if (failed(getCallOpType(rewriter.getContext(), resultType,
stridedOuterDimsAttr, callResultTypes))) {
return rewriter.notifyMatchFailure(
op, llvm::formatv("failed to lower result type {0}", resultType));
op, llvm::formatv("failed to lower result type {}", resultType));
}
}

Expand Down Expand Up @@ -242,7 +242,7 @@ struct UKernelOpsBufferizationInterface
FailureOr<Value> memrefOperand = getBuffer(rewriter, operand, options);
if (failed(memrefOperand)) {
return op->emitOpError(
llvm::formatv("failed to bufferize operand {0} ", index));
llvm::formatv("failed to bufferize operand {} ", index));
}
bufferOpOperands.push_back(memrefOperand.value());
continue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ struct LLVMCPULinkExecutablesPass

// Create our new "linked" hal.executable.
SymbolTable moduleTable(moduleOp);
std::string linkedExecutableName = llvm::formatv("{0}_linked", moduleName);
std::string linkedExecutableName = llvm::formatv("{}_linked", moduleName);
auto linkedExecutableOp = moduleBuilder.create<IREE::HAL::ExecutableOp>(
moduleOp.getLoc(), linkedExecutableName);
linkedExecutableOp.setVisibility(
Expand All @@ -55,7 +55,7 @@ struct LLVMCPULinkExecutablesPass
std::string linkedVariantName =
executableTargetAttrs.size() == 1
? targetAttr.getSymbolNameFragment()
: llvm::formatv("{0}_{1}", targetAttr.getSymbolNameFragment(),
: llvm::formatv("{}_{}", targetAttr.getSymbolNameFragment(),
index);
auto linkedTargetOp =
executableBuilder.create<IREE::HAL::ExecutableVariantOp>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -831,8 +831,8 @@ class MMTKernelGenerator {
// Perform the code replacement for the operand.
// Example: $(lhs:1) => $5
replaceAllSubstrsInPlace(
code, llvm::formatv("$({0}:{1})", name, unprocessedIdx),
llvm::formatv("${0}", processedIdx));
code, llvm::formatv("$({}:{})", name, unprocessedIdx),
llvm::formatv("${}", processedIdx));
}
};
processOperands(Constraints::Kind::InputOutput, "acc", kernel.accRegs);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ struct LLVMGPULinkExecutablesPass

// Create our new "linked" hal.executable.
SymbolTable moduleTable(moduleOp);
std::string linkedExecutableName = llvm::formatv("{0}_linked", moduleName);
std::string linkedExecutableName = llvm::formatv("{}_linked", moduleName);
auto linkedExecutableOp = moduleBuilder.create<IREE::HAL::ExecutableOp>(
moduleOp.getLoc(), linkedExecutableName);
linkedExecutableOp.setVisibility(
Expand All @@ -94,7 +94,7 @@ struct LLVMGPULinkExecutablesPass
std::string linkedVariantName =
executableTargetAttrs.size() == 1
? targetAttr.getSymbolNameFragment()
: llvm::formatv("{0}_{1}", targetAttr.getSymbolNameFragment(),
: llvm::formatv("{}_{}", targetAttr.getSymbolNameFragment(),
index);
auto linkedTargetOp =
executableBuilder.create<IREE::HAL::ExecutableVariantOp>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,15 @@ createResourceVariable(Location loc, const SubspanResourceInfo &resource,
OpBuilder builder(moduleOp.getContext());
spirv::GlobalVariableOp variable;
if (!isIndirect) {
std::string name = llvm::formatv("__resource_var_{0}_{1}_", resource.set,
resource.binding);
std::string name =
llvm::formatv("__resource_var_{}_{}_", resource.set, resource.binding);
variable = builder.create<spirv::GlobalVariableOp>(
loc, globalVariableType, name, resource.set, resource.binding);
if (resource.aliased)
variable->setAttr("aliased", builder.getUnitAttr());
} else {
std::string name =
llvm::formatv("__resource_var_indirect_{0}_", resource.set);
llvm::formatv("__resource_var_indirect_{}_", resource.set);
variable = builder.create<spirv::GlobalVariableOp>(
loc, globalVariableType, name, kIndirectBindingsSetIndex, resource.set);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ struct ConvertHalInterfaceBindingSubspan final
if (!newResultTy)
return rewriter.notifyMatchFailure(
op->getLoc(),
llvm::formatv("failed to legalize memref type: {0}", op.getType()));
llvm::formatv("failed to legalize memref type: {}", op.getType()));

auto newOp =
rewriter.replaceOpWithNewOp<IREE::HAL::InterfaceBindingSubspanOp>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ struct SPIRVLinkExecutablesPass final
std::string moduleName =
executableBuckets.size() == 1
? baseModuleName
: llvm::formatv("{0}_{1}", baseModuleName, bucketIndex);
: llvm::formatv("{}_{}", baseModuleName, bucketIndex);

LLVM_DEBUG({
llvm::dbgs() << "executable bucket #" << bucketIndex << " targets:\n";
Expand Down Expand Up @@ -179,7 +179,7 @@ struct SPIRVLinkExecutablesPass final
std::string linkedVariantName =
executableTargetAttrs.size() == 1
? attr.getSymbolNameFragment()
: llvm::formatv("{0}_{1}", attr.getSymbolNameFragment(), index);
: llvm::formatv("{}_{}", attr.getSymbolNameFragment(), index);
auto linkedTargetOp =
executableBuilder.create<IREE::HAL::ExecutableVariantOp>(
moduleOp.getLoc(), linkedVariantName, attr);
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/iree/compiler/Codegen/Utils/LinkingUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ renameWithDisambiguatedName(Operation *op, Operation *moduleOp,
int uniqueingCounter = 0;
do {
disambiguatedName =
llvm::formatv("{0}_{1}", originalName, uniqueingCounter++).str();
llvm::formatv("{}_{}", originalName, uniqueingCounter++).str();
} while (
targetSymbolMap.lookup(disambiguatedName) ||
(optionalSymbolTable && optionalSymbolTable->lookup(disambiguatedName)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ struct VMVXLinkExecutablesPass

// Create our new "linked" hal.executable.
std::string linkedExecutableName =
llvm::formatv("{0}_linked_{1}", moduleName, "vmvx");
llvm::formatv("{}_linked_{}", moduleName, "vmvx");
auto linkedExecutableOp = moduleBuilder.create<IREE::HAL::ExecutableOp>(
moduleOp.getLoc(), linkedExecutableName);
linkedExecutableOp.setVisibility(
Expand All @@ -48,7 +48,7 @@ struct VMVXLinkExecutablesPass
std::string linkedVariantName =
executableTargetAttrs.size() == 1
? targetAttr.getSymbolNameFragment()
: llvm::formatv("{0}_{1}", targetAttr.getSymbolNameFragment(),
: llvm::formatv("{}_{}", targetAttr.getSymbolNameFragment(),
index);
auto linkedTargetOp =
executableBuilder.create<IREE::HAL::ExecutableVariantOp>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,36 +225,34 @@ static void prettyPrintStatistics(const UsageInfo &usageInfo,
Statistics stats;
stats.analyze(usageInfo);

os << llvm::formatv("// Constants: {0}, ", stats.constantCount);
os << llvm::formatv("estimated storage of {0}{1} B ({2:F2} MiB)\n",
os << llvm::formatv("// Constants: {}, ", stats.constantCount);
os << llvm::formatv("estimated storage of {}{} B ({:F2} MiB)\n",
stats.constantSizeDynamic ? "minimum " : "",
stats.constantSize,
stats.constantSize / (1 * 1024 * 1024.0f));
os << llvm::formatv("// Variables: {0}, ", stats.variableCount);
os << llvm::formatv("(TBD) {0}{1} B ({2:F2} MiB)\n",
stats.variableSizeDynamic ? "minimum " : "",
stats.variableSize,
stats.variableSize / (1 * 1024 * 1024.0f));
os << llvm::formatv("// Variables: {}, ", stats.variableCount);
os << llvm::formatv(
"(TBD) {}{} B ({:F2} MiB)\n", stats.variableSizeDynamic ? "minimum " : "",
stats.variableSize, stats.variableSize / (1 * 1024 * 1024.0f));

os << llvm::formatv("// D->H Syncs: {0}\n", stats.awaitCount);
os << llvm::formatv("// D->H Syncs: {}\n", stats.awaitCount);

os << llvm::formatv("// Submissions: {0}, using cumulative ",
os << llvm::formatv("// Submissions: {}, using cumulative ",
stats.submissionCount);
os << llvm::formatv(
"{0}{1} B ({2:F2} MiB)\n", stats.transientSizeDynamic ? "minimum " : "",
"{}{} B ({:F2} MiB)\n", stats.transientSizeDynamic ? "minimum " : "",
stats.transientSize, stats.transientSize / (1 * 1024 * 1024.0f));

os << llvm::formatv("// DMA Fills: {0}\n", stats.fillCount);
os << llvm::formatv("// DMA Copies: {0}\n", stats.copyCount);
os << llvm::formatv("// Collectives: {0}\n", stats.collectiveCount);
os << llvm::formatv("// Dispatches: {0}\n", stats.dispatchCount);
os << llvm::formatv("// Async Calls: {0}\n", stats.callCount);
os << llvm::formatv("// DMA Fills: {}\n", stats.fillCount);
os << llvm::formatv("// DMA Copies: {}\n", stats.copyCount);
os << llvm::formatv("// Collectives: {}\n", stats.collectiveCount);
os << llvm::formatv("// Dispatches: {}\n", stats.dispatchCount);
os << llvm::formatv("// Async Calls: {}\n", stats.callCount);

os << llvm::formatv(
"// Executables: {0}, {1}% reuse\n", stats.executableCount,
(int)std::roundf(
(1.0f - (stats.executableCount / (float)stats.dispatchCount)) *
100.0f));
os << llvm::formatv("// Executables: {}, {}% reuse\n", stats.executableCount,
(int)std::roundf((1.0f - (stats.executableCount /
(float)stats.dispatchCount)) *
100.0f));

os << "//\n";
}
Expand Down Expand Up @@ -331,7 +329,7 @@ static void prettyPrintExecutableExportInfo(
const UsageInfo &usageInfo, IREE::Stream::ExecutableOp executableOp,
IREE::Stream::ExecutableExportOp exportOp, llvm::raw_fd_ostream &os) {
auto funcOp = exportOp.lookupFunctionRef();
prettyPrintItemHeader(llvm::formatv("stream.executable.export @{0}::@{1}",
prettyPrintItemHeader(llvm::formatv("stream.executable.export @{}::@{}",
executableOp.getName(),
exportOp.getName()),
os);
Expand Down Expand Up @@ -406,20 +404,19 @@ static void dumpAggregateCSVTable(const UsageInfo &usageInfo,
os << "\n";

// Globals:
os << llvm::formatv("{0},{1},{2},{3},", stats.constantCount,
stats.constantSize, stats.variableCount,
stats.variableSize);
os << llvm::formatv("{},{},{},{},", stats.constantCount, stats.constantSize,
stats.variableCount, stats.variableSize);

// Synchronization:
os << llvm::formatv("{0},", stats.awaitCount);
os << llvm::formatv("{},", stats.awaitCount);

// Execution:
os << llvm::formatv("{0},{1},{2},{3},{4},{5},", stats.submissionCount,
os << llvm::formatv("{},{},{},{},{},{},", stats.submissionCount,
stats.transientSize, stats.fillCount, stats.copyCount,
stats.dispatchCount, stats.callCount);

// Executables:
os << llvm::formatv("{0}", stats.executableCount);
os << llvm::formatv("{}", stats.executableCount);

os << "\n";
os << "\n";
Expand Down Expand Up @@ -452,13 +449,13 @@ static void dumpExecutionCSVTable(const UsageInfo &usageInfo,
.Case<IREE::Stream::CmdFillOp>([&](auto op) {
APInt length;
matchPattern(op.getTargetLength(), m_ConstantInt(&length));
os << llvm::formatv(R"({0},"fill",,{1},,,,)", depth, length);
os << llvm::formatv(R"({},"fill",,{},,,,)", depth, length);
os << "\n";
})
.Case<IREE::Stream::CmdCopyOp>([&](auto op) {
APInt length;
matchPattern(op.getLength(), m_ConstantInt(&length));
os << llvm::formatv(R"({0},"copy",,{1},,,,)", depth, length);
os << llvm::formatv(R"({},"copy",,{},,,,)", depth, length);
os << "\n";
})
.Case<IREE::Stream::CmdDispatchOp>([&](auto op) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ LogicalResult RegisterAllocation::annotateIR(IREE::VM::FuncOp funcOp) {
registerAllocation.remapSuccessorRegisters(terminatorOp, i);
SmallVector<std::string, 8> remappingStrs;
for (auto &srcDstReg : srcDstRegs) {
remappingStrs.push_back(llvm::formatv("{0}->{1}",
remappingStrs.push_back(llvm::formatv("{}->{}",
srcDstReg.first.toString(),
srcDstReg.second.toString())
.str());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,10 @@ LogicalResult ValueLiveness::annotateIR(IREE::VM::FuncOp funcOp) {
std::string str;
if (auto blockArg = llvm::dyn_cast<BlockArgument>(value)) {
if (blockArg.getOwner()->isEntryBlock()) {
str = llvm::formatv("%arg{0}", blockArg.getArgNumber());
str = llvm::formatv("%arg{}", blockArg.getArgNumber());
} else {
str =
llvm::formatv("%bb{0}_arg{1}", blockOrdinals[blockArg.getOwner()],
blockArg.getArgNumber());
str = llvm::formatv("%bb{}_arg{}", blockOrdinals[blockArg.getOwner()],
blockArg.getArgNumber());
}
} else {
llvm::raw_string_ostream os(str);
Expand Down
Loading

0 comments on commit b2c5f3b

Please sign in to comment.