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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
