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

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.


================
Comment at: lib/CodeGen/CGExprConstant.cpp:1413
+  } else if (!Init->isEvaluatable(CE.CGM.getContext())) {
+    return false;
+  } else if (InitTy->hasPointerRepresentation()) {
----------------
rsmith wrote:
> 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.
An important point in this patch is that CodeGen tries to find out, if the initializer can be replaced with zeroinitializer, *prior* to the evaluation of it. For huge arrays the evaluation consumes huge amount of memory and time and it must be avoided.

With this patch CodeGen may evaluate parts of the initializer, if it is represented by `InitListExpr`. It may cause redundant calculation, for instance if the check for zero initialization failed but the initializer is constant. To avoid this redundancy we could cache result of evaluation in instances of `Expr` and then use the partial values in the evaluation of the initializer. The simple use case solved by this patch probably is not a sufficient justification for such redesign.


Repository:
  rC Clang

https://reviews.llvm.org/D46241