[PATCH] D74094: Reapply: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 11 15:35:01 PDT 2023


rjmccall added a comment.

> Fuck me for trying to help, right? If x-values are part of the "basics" of parameter passing, I'm afraid to ask about the more complicated cases.

I can see how my response was somewhat aggressive, and I regret that and apologize.  I imagine you're probably approaching this from the perspective of general optimization and are unhappy that you're getting bogged down in all this C++ minutiae.  It is the nature of high-level optimization, though, that you often have to learn a lot of language details in order to get your job done.  I'm happy to teach some of these concepts during review — I wouldn't expect more than, like, twenty people on the planet to know the differences between parameter and temporary lifetimes off-hand, and I understand that non-experts in C++ aren't going to know all these parameter initialization rules by heart.  But I am surprised at some of the things we've had to cover, like that it's not generally okay for a function to return a reference to a by-value parameter.  I am volunteering my time to try to help you with your goal of landing this patch, and it can be a little frustrating to spend that time on this stuff.

If you're able to build Alex's test case, I would suggest diffing the IR output both with and without your original patch.  -O0 output is probably fine for this and will be a lot easier to track back to the IRGen code responsible for it.  You may see something that immediately stands out as wrong, but at the very least, it'll tell us what's changed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74094



More information about the cfe-commits mailing list