On Jul 10, 2013, at 4:51 PM, Faisal Vali <[email protected]> wrote: > On Wed, Jul 10, 2013 at 4:56 PM, Richard Smith <[email protected]> wrote: >> On Wed, Jul 10, 2013 at 10:06 AM, Faisal Vali <[email protected]> wrote: >>> On Wed, Jun 12, 2013 at 7:02 PM, Richard Smith <[email protected]> >>> wrote: >>>> On Sun, Jun 9, 2013 at 12:47 PM, Faisal Vali <[email protected]> wrote: >>>>> >>>>> On Fri, Jun 7, 2013 at 5:21 PM, Richard Smith <[email protected]> >>>>> wrote: >>>>>> >>>>>> On Tue, Jun 4, 2013 at 9:19 PM, Faisal Vali <[email protected]> wrote: >>>>>>> OK - this patch adds the following: >>>>>>> 0) Some codegen tests, and additional non-codegen tests. >>>>>>> 1) If the pointer to member expression can be entirely folded into a >>>>>>> constant, do so. >>>>>>> 2) If the object expression in a member access (be it through dot, or >>>>>>> pointer to member) is a constant >>>>>>> but the containing expression could not be folded into a constant - >>>>>>> do >>>>>>> not emit the object as a constant >>>>>>> - instead get the variable's address using >>>>>>> getStaticLocalDeclAddress >>>>>>> (it appears that constants are >>>>>>> hoisted out as globals in llvm IR) so that getelementptr has a >>>>>>> pointer to work with. >>>>>>> >>>>>>> Am patiently awaiting feedback - because if i am on the right track, >>>>>>> hoping >>>>>>> i can get this committed soon; if I am way off, I would like to walk >>>>>>> away >>>>>>> from this, so that I can return to working on generic lambdas ;) >>>>>> >>>>>> The Sema part looks good. >>>>>> >>>>> OK. No changes made to that portion, in this patch. >>>>> >>>>>> For the CodeGen part, I think you should approach the problem somewhat >>>>>> differently. CodeGen emits expressions as RValues or LValues based on >>>>>> how they are used, not based on whether the expressions themselves are >>>>>> rvalues or lvalues, and in particular, an operand of an >>>>>> lvalue-to-rvalue conversion is typically emitted directly as an >>>>>> rvalue. So if you ensure that RValue emission never actually performs >>>>>> a load in the cases which are not odr-uses, then you should be OK. >>>>>> >>>>>> >>>>>> One problem is that, in some cases, the rvalue emission defers to >>>>>> lvalue emission; emitting pointer-to-member access is one of those >>>>>> cases. I think what you should do here is to teach the >>>>>> ScalarExprEmitter to try to evaluate the right-hand side of the >>>>>> pointer-to-member expression, and emit a direct field access if it >>>>>> can. >>>>>> >>>>> >>>>> As per your suggestions, I have reworked the codegen portion as following: >>>>> 1) CGF::EmitIgnoredExpr checks to see if it is a constant expression, and >>>>> then >>>>> defers to RValue Emission (at least i think my change does that, am I >>>>> correct?) >>>> >>>> Please don't use isCXX11ConstantExpr here; at the IR generation level, >>>> we should give the same behavior to any expression that we can >>>> constant fold without side-effects, whether or not it's a constant >>>> expression. >>>> >>>> If the expression is constant, you don't need to emit it at all. But >>>> this change isn't correct. Consider: >>>> >>>> struct S { static const int n = 0; }; >>>> void doit(bool b, int k) { b ? k : S::n; } >>>> >>>> Since the discarded expression here isn't a constant expression, >>>> you'll emit a reference to S::n, which is not odr-used here. >>>> >>>> Instead, we could emit every discarded-value expression as an rvalue, >>>> in C++11 onwards. The extra lvalue-to-rvalue conversion is detectable >>>> iff the expression is volatile-qualified, which is exactly the >>>> situation in which we're required to emit an lvalue-to-rvalue >>>> conversion anyway. >>>> >>> >>> >>> OK. I removed the IsCXX11ConstantExpr check, and if C++11 option is on, >>> we emit as an Rvalue. >>> >>> Also, in C++11 mode, this now emits 'load volatile' for this test and it >>> fails - >>> is this the correct behavior ? >>> thoughts? >>> >>> volatile int& refcall(); >>> // CHECK: define void @_Z2f2PVi >>> // CHECK-NOT: load volatile >>> // CHECK: ret >>> void f2(volatile int *x) { >>> // We shouldn't perform the load in these cases. >>> refcall(); >>> 1 ? refcall() : *x; >>> } >> >> Hmm, OK, I was wrong to claim that we could emit every discarded-value >> expression as an rvalue; this test is correct. Here's an alternative: >> add an expression visitor for EmitIgnoredExpr to use, that implements >> the rules of 3.2/2 for discarded value expressions: Walk over >> conditionals, pointers-to-members, parens, and so on, evaluating an >> expression for its side-effects only. If you hit a DeclRefExpr, don't >> emit it. But see [1] below.
Right, I agree that causing EmitIgnoredExpr to properly ignore l-values is the right way to go. You can make more general than the 3.2/2 rules; you just need to make it at least as good. John. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
