User Details
- User Since
- Dec 30 2016, 3:24 PM (352 w, 4 d)
- Roles
- Administrator
Fri, Sep 29
Final nits
Thu, Sep 28
Wed, Sep 27
This kind of blindly applying clang-tidy -checks='-*,performance-for-range-copy' --fix changes is easy for the contributor but is time consuming for reviewers.
Anyway, I have applied appropriated fixes to places where const auto & is not the best.
Use std::size to avoid referencing vector size by integer literals
Tue, Sep 26
Are __asan_abi_register_elf_globals needed if non-Apple systems don't build lib/asan_abi?
Other than -Wframe-larger-than=656 and const uptr indicator = 0x0f0000000000ull; changes, others looks good to me.
I agree that this should be reviewed by @dvyukov :)
Mon, Sep 25
(The lld patch may take some time. I'll be out of town for about 10 days.)
The Clang Driver/CodeGen and compiler-rt changes look good to me. Of course please wait for an Android reviewer :)
LGTM
LGTM! Thanks for bearing with me about all the nits.
lld/docs/ld.lld.1 needs an update as well. The format is difficult to write... you can use man lld/docs/ld.lld.1 to check whether it looks right.
Thanks for the update.
Thanks for the update. Doing the following looks good to me.
bool CanHaveLeadingDot = true; if (StripUnderscore) if (DecoratedStr[0].consume_front("_")) // simplified CanHaveLeadingDot = false;
Sun, Sep 24
Sat, Sep 23
Fri, Sep 22
.
Instead of having a --use-*, the best is probably to create a JJ<"call-graph-profile-sort=", ...> (values: hfsort,cdsort),
make the existing --call-graph-profile-sort an alias (FF<"call-graph-profile-sort", ...>, Alias<call_graph_profile_sort>, AliasArgs<"hfsort">),
then add a BB for --no-call-graph-profile-sort.
DataLayout needs a unittest in llvm/unittests/IR/DataLayoutTest.cpp
It's very late for me and I don't have much time to create a smaller reproduce. This patch caused an assertion failure in AArch64ISelLowering.cpp:4435
assert(N->getOpcode() == ISD::BUILD_VECTOR && "expected BUILD_VECTOR");
Thu, Sep 21
llvm::demangle is used by a lot of ELF tools to assume ELF style mangling where there is no extra prefix. I think many don't expect demangling ._Zxxxx symbols.
If XCOFF needs ., I think separate functions are needed. You can add a new utility function or extending demangle with an optional parameter about XCOFF.
So this is truly a test of the ld.otherlinker feature pattern, not some special case driver feature. I guess we should leave the test alone.
Closing, we left the test alone, it still uses -fuse-ld=lld-link2. Perhaps in the future we should reconsider this, but that's how things stand now, and we aren't going to land this patch as is. + @MaskRay , who has thought about -fuse-ld= semantics.
I have also explained that SmallVector<*, 0> is preferred in lld/ELF code, so I have a vector => ArrayRef refactoring.
We should also get rid of buildCallGraph, which uses a number of output parameters, while the function can just be inlined into apply* (to be renamed).
Wed, Sep 20
Please, when landing a large patch, check whether clang can build the project. The original commit caused -Wswitch failure which I fixed in ca22d6e40508f6d24a9352835bda9c152e3eee1b.
Early heads-up we internally have identified a crash due to this change.