- User Since
- Jun 28 2018, 9:57 PM (204 w, 1 d)
Tue, May 24
Mon, May 23
Sun, May 22
Thanks for review @lkail , updated accordingly.
address @lkail comments
Tue, May 17
LGTM too. Thanks for fixing.
Mon, May 16
Thu, May 12
LGTM! Thanks for adding this amazing support.
Thu, May 5
Thanks. The PowerPC part changes are improvements and LGTM.
Wed, May 4
gentle ping for the MachineSink part. Thanks!
Apr 27 2022
Apr 26 2022
LGTM. Thanks for fixing.
LGTM with one nit.
Apr 25 2022
This patch turns on support for CR bit accesses for Power8 and above. The reason why CR bits are turned on as the default for Power8 and above is that because later architectures make use of builtins and instructions that require CR bit accesses (such as the use of setbc in the vector string isolate predicate and bcd builtins on Power10).
Apr 24 2022
The changes in GenericCycleInfo look good to me. But please do wait for reviewers looking at MachineSink.
address @sameerds comments
Apr 22 2022
Looks almost good to me. Thanks for adding this support.
Apr 20 2022
I was concerned about compile-time impact of using MachineCycleInfo, but it looks like the impact is fairly low on CTMark: http://llvm-compile-time-tracker.com/compare.php?from=42865819b22486963294434fb21b51ab3e6ebfa4&to=61706f3ee1c979e7ec893d0bc4858790561a27c0&stat=instructions Looks like SPASS with ReleaseLTO-g is the only one with some non-trivial impact.
address @sameerds comments
Apr 19 2022
Apr 8 2022
update after verifying together with functionality patch D123366
Apr 7 2022
Apr 6 2022
Mar 29 2022
Mar 21 2022
Mar 20 2022
Mar 13 2022
Mar 10 2022
Mar 9 2022
This commit also causes our bots fail https://lab.llvm.org/buildbot/#/builders/105/builds/22623
******************** TEST 'SanitizerCommon-lsan-powerpc64le-Linux :: Posix/timer.cpp' FAILED ******************** Script: -- : 'RUN: at line 1'; /home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/stage1/./bin/clang --driver-mode=g++ -gline-tables-only -fsanitize=leak -m64 -fno-function-sections -funwind-tables -ldl -O0 -g /home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/llvm/compiler-rt/test/sanitizer_common/TestCases/Posix/timer.cpp -o /home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/stage1/projects/compiler-rt/test/sanitizer_common/lsan-powerpc64le-Linux/Posix/Output/timer.cpp.tmp && /home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/stage1/projects/compiler-rt/test/sanitizer_common/lsan-powerpc64le-Linux/Posix/Output/timer.cpp.tmp -- Exit Code: 23 Command Output (stderr): -- ================================================================= ==151261==ERROR: LeakSanitizer: detected memory leaks Direct leak of 8 byte(s) in 1 object(s) allocated from: #0 0x1005410c in __interceptor_malloc.part.10 /home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/llvm/compiler-rt/lib/lsan/lsan_interceptors.cpp:75:3 #1 0x7fff81a649a8 in .annobin_timer_create.c timer_create.c #2 0x10058c24 in main /home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/llvm/compiler-rt/test/sanitizer_common/TestCases/Posix/timer.cpp:13:3 #3 0x7fff818249f4 in .annobin_libc_start.c libc-start.c #4 0x7fff81824be0 in __libc_start_main (/lib64/libc.so.6+0x24be0) (BuildId: 3f510e433e7682fc2680148fe7836ab789f8084b) SUMMARY: LeakSanitizer: 8 byte(s) leaked in 1 allocation(s).
Mar 8 2022
address @amyk comments
Mar 4 2022
I made a search in Power6 ~ Power10 ISAs, the instruction format for tlbie is like:
Mar 3 2022
My initial thought is to bail out only if the loop containing MBB or SuccToSinkTo in isProfitableToSinkTo contains irreducible cfg or not. I guess we checked the whole function instead of the single loop because LoopBlocksRPO(which is suitable to check a single loop) does not have a MachineLoop version?
Mar 2 2022
LGTM. Two nits about the comments and tests.
LGTM. Thanks for fixing this. I will check MachineCycleInfo for the main branch if you don't plan to do so.
Feb 24 2022
It should be possible to do this by switching MachineSink from using MachineLoopInfo to MachineCycleInfo, which supports irreducible cycles. I think this allows a better profitability decision, but I'm not entirely sure that it would be a sufficient correctness condition, as irreducible cycles are DFS-order dependent. In any case, MachineCycleInfo is a new addition that is not actually used anywhere yet, so I don't think this would be appropriate for an LLVM 14 backport.
Feb 23 2022
LGTM with one nit. Thanks for adding this support.
Feb 22 2022
Can we check why the instruction is sunk from a shallower block From to a deeper block To? MachineSinking::isProfitableToSinkTo() should not allow this?
Feb 20 2022
Feb 17 2022
- use . for forwarding methods
- lint error fix
LGTM too. Thanks for fixing this.
Feb 16 2022
hiding the semantics from the optimizer is sometimes a good thing and sometimes a bad thing).
Agree. Imagining a case when the neg and fma (from fnmsub) can both be CSE-ed with another neg and fma, so we can totally eliminate the fnmsub. But after we convert it to an intrinsic, we may lose the opportunity to CSE the fnmsub.
Jan 25 2022
LGTM. Thanks for adding the instructions.
Jan 24 2022
Jan 20 2022
LGTM. Please hold on some days for other reviewers. Thanks for adding this support.
Thanks for fixing this. LGTM