- User Since
- Dec 17 2014, 5:37 PM (458 w, 6 d)
Aug 24 2023
This looks reasonable to me in the sense that it treats distinct metadata as being different but bypasses distinct debug info. CC @dexonsmith
Dec 2 2021
Nov 2 2021
This commit seems to cause some regression for "-save-temps" as there is no new line before the pragma. See the below test case, -E will output
int test();#pragma clang assume_nonnull
It will fail the compilation on the preprocessed output with
error: expected unqualified-id
int test();#pragma clang assume_nonnull end
Sep 14 2021
Sep 10 2021
This looks good to me in general. Since it should not change functionality, it may not be possible to write a test case.
Jun 17 2021
Sep 11 2020
Dropping the argument sounds good to me! We are already passing the argument as undefined.
For the second, I'm less certain.
It feels cleaner to do the null test on the callee side (i.e inside the wrapper). It may have binary size advantage compared to checking at each callsite.
For the third, making it the callee's responsibility is generally better for code size but harder to optimize. But we only have the code-size cost on the caller side when materializing from a global, and that's actually something we can also eliminate as redundant. So maybe the best approach is to introduce a weak+hidden thunk that we use when making a call to a direct class method on a specific class, and we have that thunk do the entire materialization sequence.
Completely agree. For direct class method, making sure 'self' is initialized on the callee side is cleaner and better for code size. But caller has the context to know whether the class is already initialized.
Maybe we can start with doing it on callee's side and make an incremental improvement by implementing the approach that can get both benefits.
Mar 17 2020
Mar 11 2020
There are a few concerns about the approach:
1> Compile time: dumping AST as string then hashing the string. Alex measured it on a synthetic benchmark, it shows insignificant impact.
2> Stability: from my understanding, the main goal of this patch is to increase stability of the symbol name so it will not change if the relevant code for the block is not changing. It may not be as stable when the compiler version changes.
Using per-function ID improves the stability compared to per-module ID. @alexbdv do you have rough ideas on how much better the proposed approach is in terms of stability, comparing to per-function ID?
3> Using a compiler flag may slow down the adoption.
Feb 25 2020
We should add a new module flag (with type Error) that contains Major and Minor and make sure "Garbage Collection" value is the same for Swift as for ObjC.
In the backend, we will reconstruct IMAGE_INFO from the new module flags and make sure the value stays the same with and without this patch.
IMAGE_INFO will be different for Swift vs ObjC.
Just to clarify :]
Oct 29 2019
Mar 8 2019
Mar 7 2019
Mar 6 2019
Mar 5 2019
Mar 4 2019
Mar 1 2019
Mark windows as unsupported in the testing case.
Feb 28 2019
Address review comments!
Feb 27 2019
Add a testing case to make sure the instrumentation works correctly!
Feb 7 2019
I think I have addressed most of the comments. There is one comment which I am not sure.
Jan 31 2019
Address some review comments
Add support for the new pass manager
Jan 30 2019
Nov 29 2018
Mar 9 2017
Please update the patch with context: http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface
Is it possible to add a testing case?
Feb 13 2017
LGTM. Sorry for the delay,
Jan 18 2017
Hoping to wrap this up this week.
I am hoping to wrap this up this week. Thanks,
Jan 17 2017
Jan 13 2017
Add CC1 option to control hashing of the module file content.
Jan 12 2017
Bruno helped on collecting performance data for this patch:
$ cat cocoa-test.h
Addressing review comments.
Jan 11 2017
Rebase on top of r291686.
Jan 6 2017
Jan 5 2017
Add testing case.
I forgot to upload the testing case, I will add it now.
Jan 4 2017
Addressing review comments!
Dec 20 2016
Thanks for working on this, Sanjoy!
Dec 17 2016
Dec 12 2016
Dec 11 2016
Dec 7 2016
Thanks for the work!
Dec 6 2016
The rest looks pretty reasonable.
Nov 18 2016
In particular, I think once D26438 is checked in (I'm also waiting for you to take a look at that one, btw :) )
I will take a look at that soon :)
Nov 17 2016
Nov 15 2016
Nov 14 2016
We are hitting this issue when building large projects, but the reproducibility is quite low.
Nov 11 2016
Nov 10 2016
Nov 9 2016
Can you give an example where clang generates a struct path tag node where the 2nd field is not a scalar type node?