- User Since
- Nov 4 2014, 3:46 PM (282 w, 6 d)
Tue, Mar 10
Mon, Mar 9
It looks good in general. I would prefer to have two separate tests, one for upgrade, one for ir-linking.
Mar 5 2020
What is the goal for this option? Do you expect any users of this or this is for debugging WPD?
Feb 28 2020
Feb 26 2020
Thanks. LGTM. I will let @arphaman to approve this.
The 0x40 bit is actually OBJC_IMAGE_HAS_CATEGORY_CLASS_PROPERTIES, which probably can differ from translation unit to translation unit even within ObjC. Some of the other bits should not be different. That is why I say you can break the bits down even more if needed.
Clang seems to set it unconditionally. Presumably it just controls whether category descriptors have a class-properties field; offhand, I don't know why that would need to be set globally.
Feb 25 2020
I agree with all of this except that I'm not sure there's any situation where ObjC and Swift translation units should be disagreeing about the ObjC-specific parts of the OBJC_IMAGE_INFO.
Feb 24 2020
I don't feel like this is necessary and creating a subdirectory without creating a new library feels weird. I agree the layout is a bit weird for new comer to LTO library but it is a relatively small hurdle. I don't exactly feel like I want to pay the cost of making git blame a bit harder.
Feb 20 2020
Feb 19 2020
I forgot if there is reason to use the option by default at all time (I did ask that in the previous review but Alex might have given more context offline).
Feb 7 2020
I see. I don't really care then. LGTM!
Feb 6 2020
I hope we are not renaming everything because it add complexity on compatibility.
Feb 2 2020
The only case I know which can break after ABI change is the application has its own implementations to inspecting and handling exceptions. Almost everything go through libunwind and libcxxabi so as long as they are agree on the ABI, you have most of the cases covered.
I don't really mind either way. If you are using clang, you are really not expecting to use <unwind.h> in libunwind without going through the clang header, which has both versions defined.
Feb 1 2020
Looks like this is an ABI change for WIN64. If that is intended then LGTM
Jan 30 2020
@hans I landed this on master. Let me know if I need to do something for release branch. I will wait a bit to see if it breaks any unexpected users.
Jan 29 2020
Move static_assert into header
Jan 28 2020
It was updated locally on my side but I guess it won't reflected here. Updating.
Conditionalize the .private_extern based on VISIBILITY_HIDDEN macro
Jan 23 2020
Jan 22 2020
@rsmith This also breaks macOS SDK. Can we revert this as we discuss a less aggressive option?
Jan 20 2020
Jan 10 2020
Dec 19 2019
- I will add a llvm-link tests for combining objc and swift bitcode.
- What do you mean by "codegen tests for correct value in the object file"? There is no "object file" in the picture. Are you asking for a test that checks the value in the output of llvm-link?
Dec 18 2019
Commited in: 9366397f057d18401e680b2cb28a0ee17c59d4a6
LLVM LTO part LGTM. I will get another pair of eyes on lld changes.
Dec 16 2019
This sounds like a good idea. Have you thought about just write out upgraded bitcode file on disk (since you named it a "SymTabCache"), maybe into thinLTO cache? This will speed up the incremental build as well. I think most of the upgrade path are coming from linking a bitcode static library, which is probably not changing during incremental build so we just need to upgrade once.
The fix LGTM. Do you have a reproducer that can be used as a testcase? We should really add more tests for libunwind.
Dec 13 2019
Address review feedback
Dec 12 2019
Dec 10 2019
Dec 9 2019
Breaking part the meta data sounds like a good idea. You will probably need to combine the value in the backend to generate the field in the object file.
Thanks for increasing test coverage. One comment in line.
Ah, I miss your fix. The fix LGTM.
Dec 5 2019
Dec 4 2019
Dec 2 2019
Nov 21 2019
Nov 20 2019
Nov 19 2019
LGTM with one nitpick.
Nov 18 2019
Address review feedback
Nov 14 2019
Nov 8 2019
I think this is probably the best fix I can think as well. Agree with Teresa says, please add comments for reasons why it is implemented this way.
Nov 6 2019
Nov 5 2019
Oct 30 2019
Oct 29 2019
I can see why this is a good idea, but do you have numbers for this optimization? How much code can be inlined? Benchmark for speedup? code size? compile time impact?
Oct 24 2019
ccache does not have the support for this, I am just saying that this can be easily implemented in ccache and that would be much better than the proposed solution here.
Oct 23 2019
Can you clarify what do you mean by 'waste time optimizing a file that finally hit the object file cache'?
Oct 21 2019
That is not exactly what I meant. I meant: