Fixed string memory leak

[Mr-Anyone]

Motivation

Prevent memory leak and hard to debug situations.

Technical Details

Fix memory leak making a container like llvm::SmallVector take ownership.

Resolve https://github.com/ROCm/rocMLIR-internal/issues/1987

Test Plan

Builds locally and wait for CI.

Test Result

Build locally.

Submission Checklist

• Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

[Mr-Anyone]

I'll try to find more of these type of issue running grep in the codebase with SmallString.

[Mr-Anyone]

Also, it seems that defineDim makes a copy of llvm::StringRef as well.

rocMLIR/mlir/lib/Dialect/Rock/IR/TransformMapBuilder.cpp

Lines 292 to 307 in 687737e

	void TransformMapBuilder::defineDim(StringRef name, uint32_t dim,
	int64_t size) {
	assert(!frozen && "It's a bug to add to a coordinate transform after "
	"fetching the attribute");
	bool nameInsertResult = endIndices.insert({name, dim}).second;
	assert(nameInsertResult &&
	"Trying to redefine a result name in a coordinate transform");
	SmallString<8> nameCopy = name;
	bool dimInsertResult = endNames.insert({dim, nameCopy}).second;
	assert(dimInsertResult &&
	"Trying to redefine a result dimension in a coordinate transform");
	for (uint32_t e = endShape.size(); e <= dim; ++e) {
	endShape.push_back(0);
	}
	endShape[dim] = size;
	}

[justinrosner]

I'll try to find more of these type of issue running grep in the codebase with SmallString.

I think you'd want to grep for uses of addDim, and other TransformMapUtils like it that take in a string, instead of just looking for SmallString.

[dhernandez0]

it can be a separate PR, but it'd be nice to change TransformMapBuilder and subclasses to not take "StringRef" to avoid these kind of issues.

[Mr-Anyone]

I have grep the following and made the changes:

\.passThrough\(
\.constDim\(
\.embed\(
\.unmerge\(
\.merge\(
\.takeRemainder\(
\.addDim\(
\.dropDimAtIndex\(
\.dropDimsAtIndices\(
\.slice\(

[justinrosner]

it can be a separate PR, but it'd be nice to change TransformMapBuilder and subclasses to not take "StringRef" to avoid these kind of issues.

@Mr-Anyone Can you make sure that we have a ticket open in our backlog to properly addresses changing the API for these transform map function?