[CIR] Add builtin operator new/delete

[HendrikHuebner]

This PR adds __builtin_operator_new and __builtin_operator_delete. The implementation is taken from clang code gen.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clangir

Author: Hendrik Hübner (HendrikHuebner)

Changes

This PR adds __builtin_operator_new and __builtin_operator_delete. The implementation is taken from clang code gen.

---

Full diff: https://github.com/llvm/llvm-project/pull/168578.diff

4 Files Affected:

• (modified) clang/include/clang/CIR/MissingFeatures.h (+1)
• (modified) clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp (+8)
• (modified) clang/lib/CIR/CodeGen/CIRGenExprCXX.cpp (+27)
• (modified) clang/lib/CIR/CodeGen/CIRGenFunction.h (+3)

diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h
index 567c79a27c07b..477d8046e18c0 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -200,6 +200,7 @@ struct MissingFeatures {
   static bool aggValueSlotMayOverlap() { return false; }
   static bool aggValueSlotVolatile() { return false; }
   static bool alignCXXRecordDecl() { return false; }
+  static bool allocToken() { return false; }
   static bool appleKext() { return false; }
   static bool armComputeVolatileBitfields() { return false; }
   static bool asmGoto() { return false; }
diff --git a/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp b/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp
index 77f19343653db..2fbad2ecce0d3 100644
--- a/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp
@@ -18,6 +18,7 @@
 #include "mlir/IR/BuiltinAttributes.h"
 #include "mlir/IR/Value.h"
 #include "mlir/Support/LLVM.h"
+#include "clang/AST/DeclBase.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/GlobalDecl.h"
 #include "clang/Basic/Builtins.h"
@@ -520,6 +521,13 @@ RValue CIRGenFunction::emitBuiltinExpr(const GlobalDecl &gd, unsigned builtinID,
     cir::PrefetchOp::create(builder, loc, address, locality, isWrite);
     return RValue::get(nullptr);
   }
+  case Builtin::BI__builtin_operator_new:
+    return emitNewOrDeleteBuiltinCall(
+        e->getCallee()->getType()->castAs<FunctionProtoType>(), e, false);
+  case Builtin::BI__builtin_operator_delete:
+    emitNewOrDeleteBuiltinCall(
+        e->getCallee()->getType()->castAs<FunctionProtoType>(), e, true);
+    return RValue::get(nullptr);
   }
 
   // If this is an alias for a lib function (e.g. __builtin_sin), emit
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprCXX.cpp b/clang/lib/CIR/CodeGen/CIRGenExprCXX.cpp
index 007d873ff5db6..f28887df34212 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprCXX.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprCXX.cpp
@@ -610,6 +610,33 @@ static RValue emitNewDeleteCall(CIRGenFunction &cgf,
   return rv;
 }
 
+RValue CIRGenFunction::emitNewOrDeleteBuiltinCall(const FunctionProtoType *type,
+                                                  const CallExpr *callExpr,
+                                                  bool isDelete) {
+  CallArgList args;
+  emitCallArgs(args, type, callExpr->arguments());
+  // Find the allocation or deallocation function that we're calling.
+  ASTContext &astContext = getContext();
+  DeclarationName name = astContext.DeclarationNames.getCXXOperatorName(
+      isDelete ? OO_Delete : OO_New);
+
+  clang::DeclContextLookupResult lookupResult =
+      astContext.getTranslationUnitDecl()->lookup(name);
+  for (const auto *decl : lookupResult) {
+    if (const auto *funcDecl = dyn_cast<FunctionDecl>(decl)) {
+      if (astContext.hasSameType(funcDecl->getType(), QualType(type, 0))) {
+        // Used for -fsanitize=alloc-token
+        assert(!cir::MissingFeatures::allocToken());
+
+        // Emit the call to operator new/delete.
+        return emitNewDeleteCall(*this, funcDecl, type, args);
+      }
+    }
+  }
+
+  llvm_unreachable("predeclared global operator new/delete is missing");
+}
+
 namespace {
 /// Calls the given 'operator delete' on a single object.
 struct CallObjectDelete final : EHScopeStack::Cleanup {
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h
index 00f289bcd1bb2..0cfcc2be0255e 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -1476,6 +1476,9 @@ class CIRGenFunction : public CIRGenTypeCache {
 
   RValue emitCXXPseudoDestructorExpr(const CXXPseudoDestructorExpr *expr);
 
+  RValue emitNewOrDeleteBuiltinCall(const FunctionProtoType *type,
+                                    const CallExpr *call, bool isDelete);
+
   void emitCXXTemporary(const CXXTemporary *temporary, QualType tempType,
                         Address ptr);
 

[HendrikHuebner]

OGCG actually generates a bunch of attributes, something like: call noalias noundef nonnull ptr @_Znwm(i64 noundef 4).

Is it okay to ignore these with the wildcard syntax in the test?

[xlauko]

Where are these set? And why is it not set in CIR pipeline?
I would prefer not to ignroe them in this case as CIRGen is creating the call in this case?

[andykaylor]

I think they're being set in CodeGenModule::ConstructAttributeList in classic codegen. This is a known hole in CIR codegen. Even if we were setting these the same as OGCG, I'd want to use a wildcard match here.

[github-actions]

🐧 Linux x64 Test Results

• 112034 tests passed
• 4077 tests skipped

[xlauko]

lgtm % nits

[xlauko]

add comments /*ParamName=*/ for boolean parameters

[andykaylor]

Better still, you could replace the boolean parameter with OverloadedOperatorKind and pass OO_New or OO_Delete since the only thing the parameter is used for is to select between these two.

[xlauko]

Where are these set? And why is it not set in CIR pipeline?
I would prefer not to ignroe them in this case as CIRGen is creating the call in this case?

[bcardosolopes]

This should be wrapped in a check like:

          if (SanOpts.has(SanitizerKind::AllocToken)) {
            // Set !alloc_token metadata.
            assert(!cir::MissingFeatures::allocToken());
            errorNYI(...)
          }

If someone is using -fsanitize=alloc-token we should error, silent passing here could be super misleading. Note that some existing sanitizer checks we are not currently leading to errorNYI, so this isn't a strict rule necessarily, but it involving extra allocations and builtins justify the importance here IMO

[andykaylor]

We've been wildly inconsistent about that, but I agree that we should do better.

[HendrikHuebner]

Done