[CIR] Upstream support array new

[andykaylor]

Overview

Upstream support for array new handling.

Suggested minimal test case

void t_new_constant_size() {
  auto p = new double[16];
}

Existing incubator tests

clang/test/CIR/CodeGen/new.cpp
clang/test/CIR/CodeGen/array-new-init.cpp
clang/test/CIR/CodeGen/applearm64-array-cookies.cpp

[jiang1997]

I'd like to give this a try. It seems I should add some code to the function below to support array new
expressions. Is that right?

llvm-project/clang/lib/CIR/CodeGen/CIRGenExprCXX.cpp

Lines 213 to 226 in 1b944e2

	static mlir::Value emitCXXNewAllocSize(CIRGenFunction &cgf, const CXXNewExpr *e,
	unsigned minElements,
	mlir::Value &numElements,
	mlir::Value &sizeWithoutCookie) {
	QualType type = e->getAllocatedType();
	mlir::Location loc = cgf.getLoc(e->getSourceRange());
	if (!e->isArray()) {
	CharUnits typeSize = cgf.getContext().getTypeSizeInChars(type);
	sizeWithoutCookie = cgf.getBuilder().getConstant(
	loc, cir::IntAttr::get(cgf.SizeTy, typeSize.getQuantity()));
	return sizeWithoutCookie;
	}

[andykaylor]

@jiang1997 This is a bit bigger task than it may appear, but if you want to take a shot at it, that would be great.

The code you linked is the correct starting point. This happens to be a bit of code that I implemented in the incubator, so I can point you to the first PR I put up to support it there: llvm/clangir#1286

The first PR upstream should look very much like that, but there will be some changes needed. Instead of having a separate "lowering" test, we're using three sets of checks (CIR, LLVM, and OGCG) in the upstream CodeGen tests. Most existing CodeGen tests can be used as an example of the RUN lines you'll need.

You'll also need to update the variable names to match the identifier naming convention we're following in the upstream code. If you use clangd it should tell you what needs to be changed. Otherwise, the expectations are documented here: https://llvm.github.io/clangir/GettingStarted/coding-guideline.html

You should start with the code currently in the incubator rather than the code in the PR I linked above as there may have been changes made since then, but you should use that PR as a guide to where to separate the code for upstreaming. In the incubator we use llvm_unreachable("NYI"); where code is omitted, but upstream we use cgm.errorNYI([SourceLocation], [message]); for the same purpose.

You can find about seven more PRs that incrementally added to this feature at this link: https://github.com/llvm/clangir/pulls?page=2&q=is%3Apr+is%3Aclosed+author%3Aandykaylor Those can serve as a guide to more things that need to be done. However, you'll find that I didn't completely finish this in the incubator before changing my focus to upstreaming, so even after all the code from the incubator has been upstreamed, you'll find there's still a bit more to be done.

[jiang1997]

@andykaylor Thank you for the detailed explanation! It looks like all I need to do is adapt the implementation from the incubator project to the upstream and maybe complete the remaining work.

Here are the PRs I have identified based on your guide that need to be adopted. Excited to get started!

• [CIR] Add limited support for array new clangir#1286
• [CIR] Add initial support for array cookies clangir#1297
• [CIR][CIRGen] Implement array cookie ABI for AppleARM64 targets clangir#1301
• Implement support for zero-initialized builtin type allocation clangir#1304
• [CIR] Implement partial initialization for array new clangir#1307
• [CIR] Implement array new handling for variable array size clangir#1313
• [CIR] Add support for array new with constructors clangir#1347
• [CIR] Add handling for initializing multi-dimension arrays clangir#1369

[jiang1997]

Hi, @andykaylor
This issue seems to have been closed with incomplete tasks remaining. Was this intentional?

[andykaylor]

Hi, @andykaylor This issue seems to have been closed with incomplete tasks remaining. Was this intentional?

No, sorry. I'm not sure what happened. I didn't mean to close it.