rsmith added inline comments.

================
Comment at: lib/CodeGen/CGExprConstant.cpp:1414-1415
+    Expr::EvalResult Result;
+    if (Init->EvaluateAsRValue(Result, CE.CGM.getContext()) &&
+        !Result.hasUnacceptableSideEffect(Expr::SE_NoSideEffects))
+      return (Result.Val.isInt() && Result.Val.getInt().isNullValue()) ||
----------------
sepavloff wrote:
> rsmith wrote:
> > Please call `D.evaluateValue()` here rather than inventing your own 
> > evaluation scheme. That way, we'll cache the evaluated initializer on the 
> > variable for other uses or reuse the value if we've already evaluated it, 
> > and you don't need to worry about the corner cases involved in getting the 
> > evaluation right. (As it happens, you're getting some minor details wrong 
> > because evaluating an initializer is not quite the same as evaluating an 
> > rvalue, but in practice it's not a big deal here.)
> 
> Call of `D.evaluateValue()` may result in substantial memory and time 
> consumption if the variable value is huge, like in:
> ```
> int char data_1[2147483648u] = { 0 };
> ```
> The idea of this patch is to recognize some cases of zero initialization 
> prior to the evaluation of variable initializer. In the example above, value 
> would be evaluated only for part of the initializer, namely `0`, which does 
> not have an associated variable, so call of `D.evaluateValue()` is not 
> possible.
> 
As noted, `EvaluateAsRValue` gets some of the details here wrong. I reverted 
this in r332886 because of a miscompile due to this fact.


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

Reply via email to