[PATCH] CodeGen: Emit constant array temporaries into read-only globals.

Richard Smith richard at metafoo.co.uk
Tue Mar 3 12:24:28 PST 2015


I support the direction here, but this patch is non-conforming. Please disable this under `-fno-merge-all-constants`, as we do for the similar case with a const local variable. (Generally, I think this should match what we do for const local variables to the extent possible.)


================
Comment at: lib/CodeGen/CGExpr.cpp:308
@@ +307,3 @@
+    // instructions.
+    if (M->getType()->isArrayType() &&
+        CGF.CGM.isTypeConstant(M->getType(), true))
----------------
For const locals, we also do this "optimization" for objects of record type (see `CodeGenFunction::EmitAutoVarAlloca`); any reason not to do the same here?

================
Comment at: lib/CodeGen/CGExpr.cpp:312
@@ +311,3 @@
+              CGF.CGM.EmitConstantExpr(Inner, M->getType(), &CGF)) {
+        assert(Inner->isConstantInitializer(CGF.getContext(), false));
+        auto *GV = new llvm::GlobalVariable(
----------------
I don't think this assert is correct. For any expression where `isConstantInitializer` returns `true`, we should be able to emit the expression as a constant (well, even here there are known bugs which we work around by checking the type is POD), but I don't think the reverse is generally true. If CodeGen finds some unexpected way to emit an expression as a constant, that's OK.

================
Comment at: lib/CodeGen/CGExpr.cpp:315
@@ +314,3 @@
+            CGF.CGM.getModule(), Init->getType(), /*isConstant=*/true,
+            llvm::GlobalValue::PrivateLinkage, Init, ".initializer_list");
+        GV->setAlignment(
----------------
This name doesn't make sense. All you know is that it's an array temporary.

http://reviews.llvm.org/D8034

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list