User Details
- User Since
- Nov 12 2014, 1:58 PM (435 w, 5 d)
Feb 4 2023
Steven, Let's discuss this offline and come back with a proposal.
Jan 25 2023
LGTM
Jan 11 2023
LGTM
Dec 22 2022
@MaskRay Roy hasn't replied. We found other spellings that break (e.g. -Xcctests or something). Revert this patch until we find an agreement.
Dec 16 2022
Fine if Roy is happy. If we find another thing that break, we'll let you know. Please wait for him to comment.
Dec 15 2022
LGTM
Dec 13 2022
Dec 9 2022
Dec 8 2022
@t.p.northover regardless of whether this meets the bar, this is a test case that takes forever reduced from a real project.
We should have a solution upstream and this is the purpose of the patch. Downstream can't live with hacks forever.
Do you have any thoughts?
Dec 7 2022
An interesting alternative might be limiting this to when we're optimizing for size. Not sure if it sounds good for you, but maybe it's better than a cl::opt.
Dec 1 2022
LGTM, again.
Nov 30 2022
LGTM with the comment reworded. Thanks.
Exprs.size() == 1 is still valid in every example we've seen, so yes, you might want to keep it (and update the assertion message)
@aeubanks Gentle ping
Nov 28 2022
Nov 23 2022
To elaborate further, the testcase in the project issue reported is reduced from a preprocessed file that's approximately 1 million lines.
Even if people would like the remove always_inline -- it's very hard to understand where to start (and, realistically, it's a jenga tower of optimizations + templates).
Hi @aeubanks -- admittedly I haven't touched the inliner in a very long time so I might be wrong here, but my understanding is that alwaysinline trumps the cost multiplier (that's why the previous patch is not enough).
Hey folks, this is still not enough. We're seeing exponential behaviors when linking real-world projects.
Nov 22 2022
Nov 16 2022
Apr 27 2022
LGTM
Compact unwind was developed as a goal to replace the DWARF unwind. Some apps did not work if the OS was missing DWARF unwind, so we kept both for Intel. The binary compatibility issue is non-existent on arm64, but it could be still a thing on x86_64.
If a function cannot be encoded in CU, we need the compiler to emit both (where the CU is a magic value that points to the dwarf unwind info for that function).
Mar 7 2022
I don't run POSIX compliance, somebody just gave me a patch, but yes, good point.
We fixed this in the linker.
Nov 8 2021
Oct 22 2021
Aug 6 2021
May 19 2021
Apr 27 2021
Apr 20 2021
Mar 22 2021
removing davide
+ steven
Mar 17 2021
Sure.
Mar 11 2021
+ Lang, who probably has a better understanding of this, for visibility.
I don't think this should be blocked further, but he might have a chance to comment.
LGTM.
Feb 26 2021
Jan 11 2021
Dec 7 2020
Dec 1 2020
I apologize, but I don't work on this anymore.
Oct 23 2020
Oct 12 2020
I'm sorry, I haven't touched this code in a long time and I don't feel qualified to review it.
Sep 21 2020
Committed thusly
Sep 14 2020
Sep 3 2020
This landed in a different form.
Somebody will pick this up, probably in a different form.
I won't have time to look at this in the future.
Aug 27 2020
Aug 24 2020
@labath something I noticed when finding this (and related bugs) is that frame var carries a decent diagnostic
I came to the same conclusion when analyzing https://bugs.llvm.org/show_bug.cgi?id=47257 (but you beat me to the punch).
Aug 21 2020
I think this is fine. @mehdi_amini ?
Aug 14 2020
Aug 11 2020
Aug 5 2020
This is correct to the best of my understanding. Thank you Jason.
yes, this makes sense. We could refine the check in future.
I assume we're still allowing to put the decorator on a test-by-test basis, and that seems the case from what I see.
If so, LG.
Aug 4 2020
LGTM with Saleem's comment addressed.
Aug 3 2020
Jul 30 2020
Reviewed by Jason privately.
Added the check that Jason requested.
Jul 28 2020
Florian, can you take a look?
Jul 22 2020
why? Do you have a testcase?
yeah, I think it's reasonable.