[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: lib/CodeGen/CGExprConstant.cpp:1413
+  } else if (!Init->isEvaluatable(CE.CGM.getContext())) {
+    return false;
+  } else if (InitTy->hasPointerRepresentation()) {
----------------
sepavloff wrote:
> rjmccall wrote:
> > Aren't the null-pointer and integer-constant-expression checks below already checking this?  Also, `isEvaluatable` actually computes the full value internally (as an `APValue`), so if you're worried about the memory and compile-time effects of producing such a value, you really shouldn't call it.
> > 
> > You could reasonably move this entire function to be a method on `Expr` that takes an `ASTContext`.
> Comment for `EvaluateAsRValue` says that it tries calculate expression agressively. Indeed, for the code:
> ```
>   decltype(nullptr) null();
>   int *p = null();
> ```
> compiler ignores potential side effect of `null()` and removes the call, leaving only zero initialization. `isNullPointerConstant` behaves similarly.
Nonetheless, it looks like this function could evaluate `Init` up to three times, which seems unreasonable. Instead of the checks based on trying to evaluate the initializer (`isNullPointerConstant` + `isIntegerConstantExpr`), how about calling `VarDecl::evaluateValue()` (which will return a potentially pre-computed and cached initializer value) and checking if the result is a zero constant?

In fact, `tryEmitPrivateForVarInit` already does most of that for you, and the right place to make this change is probably in `tryEmitPrivateForMemory`, where you can test to see if the `APValue` is zero-initialized and produce a `zeroinitializer` if so. As a side-benefit, putting the change there will mean we'll also start using `zeroinitializer` for zero-initialized subobjects of objects that have non-zero pieces.


Repository:
  rC Clang

https://reviews.llvm.org/D46241