lebedev.ri added inline comments.

================
Comment at: lib/CodeGen/CGExprScalar.cpp:1694
 // handle things like function to ptr-to-function decay etc.
 Value *ScalarExprEmitter::VisitCastExpr(CastExpr *CE) {
   Expr *E = CE->getSubExpr();
----------------
vsk wrote:
> lebedev.ri wrote:
> > vsk wrote:
> > > I think maintaining a stack of visited cast exprs in the emitter be 
> > > cheaper/simpler than using ASTContext::getParents. You could push CE here 
> > > and use a RAII helper to pop it. The 'isCastPartOfExplicitCast' check 
> > > then simplifies to a quick stack traversal.
> > Hmm, two things come to mind:
> > 1. This pessimizes the (most popular) case when the sanitizer is disabled.
> > 2. `ASTContext::getParents()` may return more than one parent. I'm not sure 
> > if that matters here?
> > I'll take a look..
> As for (1), it's not necessary to push/pop the stack when this sanitizer is 
> disabled. And for (2), IIUC the only interesting case is "explicit-cast 
> <implicit-cast>+", and none of the implicit casts here have more than one 
> parent.
> I think maintaining a stack of visited cast exprs in the emitter be 
> cheaper/simpler than using ASTContext::getParents. 

{F6660377}

So yeah, this could work.
Except sadly it breaks in subtle cases like https://godbolt.org/g/5V2czU
I have added those tests beforehand.

Is `ASTContext::getParents()` really horribly slow so we want to 
duplicate/maintain/track
the current AST stack ourselves?
If so, we will need to maintain the *entire* stack, not just `CastExpr''s...


Repository:
  rC Clang

https://reviews.llvm.org/D48958



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to