rjmccall added a comment.

> That's better than the status quo (no lifetime markers at all) but still 
> suboptimal, and I don't think it will solve the particular case I care about 
> (a particular Linux kernel driver written in C which is passing many 
> aggregates by value).

Ah, all in the same full-expression?  Then yeah, my idea wouldn't be good 
enough to optimize your situation.

> Do I want to create a flag for that? Not particularly; it does mean 
> supporting BOTH codegen patterns; I'd rather have a flag to control the 
> optimization or not do it at all rather than support 2 different 
> optimizations.

The received wisdom here is that it's very helpful to have a flag because it 
lets users test the effect of different changes independently.  Users mostly 
don't live on trunk, and LLVM releases are roughly every six months, so when 
users get a new compiler it's going to have at least six months of accumulated 
changes in it.  If a project breaks with a new compiler, it's really great to 
be able to isolate which change is causing the problem without having to 
rebuild the compiler.

The fact that we have dynamic checking of this from ASan does help a lot, 
though.

> Personally, I feel that "we found a buggy example of code we broke, we fixed 
> it. Let's reland it and tell people to be careful when upgrading." But I 
> haven't been around long enough to know what's the precedent here for 
> "aggressive optimizations." Do you feel strongly that we should not just 
> reland this, @rjmccall (or anyone else)?

That several things broke immediately after commit does suggest that there are 
quite a bit more project bugs out there waiting to blow up.  I think having a 
dedicated flag for just this optimization is probably a good idea in the short 
term.

> One thing I noticed, simply printing the expression when we were about to add 
> the lifetime cleanup; I noticed some examples would trigger on 
> CXXConstructExpr, CXXTemporaryObjectExpr, and CXXOperatorCallExpr. Those feel 
> very C++ specific; I do wonder if we could do something differently for C, 
> but I think we should not since as @rjmccall said "we'd have a consistent 
> rule for all types and targets" which IMO is nicer+simpler.

Well, if the language rules are different, they're different.  (More on that in 
a second.)  As far as I know, the lifetime of a parameter variable in C is 
never specified as outliving the call, and the entire concept of a 
full-expression is a C++ism.  We could do something more aggressive in C.

> Converting the above widget example to C, I noticed that we're also missing 
> the lifetime markers on compound literals!

Yeah, so this is probably because the lifetime of a compound literal is 
somewhat surprising in C: it's actually the containing block scope.  We could 
emit lifetime markers, but we'd have to make sure we emitted them in the right 
place.  In pure C, I think this is only detectable if the user actually takes 
the address of the literal; unlike C++, C doesn't implicitly expose the address 
of temporaries or have destructors that allow their lifetime to be directly 
tested.  But in ObjC, you could have a compound literal with weak/strong 
references in it, which arguably are required to be destroyed at the close of 
the enclosing block.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74094/new/

https://reviews.llvm.org/D74094

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

Reply via email to