- User Since
- Apr 11 2019, 6:15 AM (19 w, 6 h)
Sat, Aug 17
Fri, Aug 16
LGTM with a minor style nit.
Thu, Aug 15
Hi Sam, this patch seems to have caused a correctness regression in the telecomm-gsm and toast benchmarks when compiled for Arm. Logs at http://lab.llvm.org:8011/builders/clang-native-arm-lnt-perf/builds/170/. Would you mind looking into this?
Wed, Aug 14
Fri, Aug 9
Mon, Aug 5
Fri, Aug 2
- Switch to using ArrayRef instead of null-terminated array
Thu, Aug 1
- Add LTOPostLink metadata, instead of internalising vcall visibility at LTO time
Partial linking will indeed prevent dropping the virtual functions, but it should not prevent clearing the pointer to the virtual function in the vtable. The linker should then be able to drop the virtual function body as part of --gc-sections during the final link.
Wed, Jul 31
- Take the example from my earlier message, give the "main executable" and "DSO" hidden visibility, build the "main executable" with LTO and the "DSO" without LTO, and statically link them both into the same executable. We run into the same problem where the Plugin1 vtable is potentially not referenced and thus misoptimised. Yes, this is a violation of the LTO visibility rules, but the example shows that we only detect it sometimes. I think that if we did want to detect cases where the LTO visibility rules are clearly being violated, the outcome should be to issue a diagnostic and not to silently proceed with optimizations disabled, since the violation might be masking other undetected issues. That really seems orthogonal to this patch, though.
Tue, Jul 30
- Don't emit llvm.assume when not necessary (we already weren't checking for it's presence in GlobalDCE)
- s/"public"/"default"/ in IR docs
Out of interest, why does the ABI allow functions which don't have SVE args/returns to clobber the P registers? For Z registers, we've got to be compatible with old code which only needed to save the bottom half of v8-v15, but there should be no existing code which uses P registers, so we could enforce a mixture of callee- and caller-saved P registers for all code. Existing code is already compliant with this, because it doesn't touch the P regs.
In that example, with everything having default ELF visibility, all of the vtables will get vcall_visibility public, which can't be optimised by VFE, and won't ever be relaxed to one of the tighter visibilities.
Mon, Jul 29
Would this also be profitable for Thumb2?
Thu, Jul 25
Ok, but please add a comment, probably in AArch64.td, explaining this, and that turning it on by default is only temporary.
This patch LGTM now, and +1 to the idea of documenting the attacks we do/don't expect to be able to defend against.
Wed, Jul 24
Jul 22 2019
Jul 19 2019
Jul 18 2019
Split into separate functions for safe/profitable.
Jul 16 2019
Jul 12 2019
Two of the new tests are failing on the Arm and AArch64 buildbots - see for example http://lab.llvm.org:8011/builders/libcxx-libcxxabi-libunwind-armv7-linux/builds/946/steps/test.libcxx/logs/FAIL%3A%20libc%2B%2B%3A%3Adeduct.fail.cpp
Jul 11 2019
Jul 10 2019
All of the libcxx bots are currently failing with this compile error:
libcxx/src/exception.cpp:14:12: fatal error: 'cxxabi.h' file not found
In case you haven't noticed it yet, the aarch64 bots are now failing with an undefined symbol __tsan::__interception::real_setjmp: http://lab.llvm.org:8011/builders/clang-cmake-aarch64-lld/builds/6879/steps/ninja%20check%201/logs/stdio
What about the tests for large stack frames?
Jul 9 2019
Actually, I think cross-unwinding with EHABI is already completely broken, because we are doing raw memory accesses (not using the AddressSpace), so this is fine.
This won't work for cross-unwinding between a big and little endian system, I think it would be better to load the whole word, and mask out the correct byte.
Thanks, that's fixed the link errors, but we are now seeing some failures in TSan tests (http://lab.llvm.org:8011/builders/clang-cmake-aarch64-lld/builds/6874, http://lab.llvm.org:8011/builders/clang-cmake-aarch64-lld/builds/6874/steps/ninja%20check%201/logs/FAIL%3A%20ThreadSanitizer-aarch64%3A%3A%20fiber_longjmp.cc). Could these be related to one of your patches?
Jul 8 2019
LGTM with one minor nit.
We are still seeing liner errors in the AArch64 build bots (http://lab.llvm.org:8011/builders/clang-cmake-aarch64-full/builds/7487/steps/ninja%20check%202/logs/stdio), are you looking into this? If there's not a quick, obvious fix, it would be better to revert the original commit to get the bots back to green.
There are a number of buildbot failures which look related to this, e.g.
Jul 5 2019
I think this could do with some more tests covering different stack layouts. In particular, we should check that we do the correct thing for functions which use frame or base pointers, and when large stack frames cause the immediates in STG and ADDG instructions go out of range.
Jul 4 2019
LGTM with one nit.
Jul 3 2019
- Add a PhysReg parameter to ignoreCSRForAllocationOrder
- Check that the register is a GPR in the ARM implementation. The other register classes have the callee-saved regs last, so this doesn't make any difference to the generated code, but might avoid surprising behaviour in the future.