[PATCH] D113424: ext-tsp basic block layout

Thu Oct 12 07:30:54 PDT 2023

kosarev added inline comments.

================
Comment at: llvm/lib/Transforms/Utils/CodeLayout.cpp:404-405
+    Result.insert(Result.end(), Begin1, End1);
+    Result.insert(Result.end(), Begin2, End2);
+    Result.insert(Result.end(), Begin3, End3);
+    return Result;
----------------
spupyrev wrote:
> kosarev wrote:
> > spupyrev wrote:
> > > kosarev wrote:
> > > > This seems to fail on expensive checks, see <https://github.com/llvm/llvm-project/issues/68594>.
> > > > 
> > > > Do `{Begin,End}{2,3}` really form valid ranges when default to `BlockIter()` in the constructor?
> > > Can you teach me to reproduce the failure? I'm building with "-DCMAKE_BUILD_TYPE=Debug -DLLVM_ENABLE_ASSERTIONS=ON" and run "ninja check-all" but still don't see it
> > It fails on libc++'s consistency checks, which get enabled automatically on builds with LLVM's expensive checks turned on, `-DLLVM_ENABLE_EXPENSIVE_CHECKS=ON`.
> hmm. I still cannot reproduce this. Here is my command:
> ```
> cmake \
>     -G Ninja \
>     -DCMAKE_ASM_COMPILER=$MYCLANG \
>     -DCMAKE_ASM_COMPILER_ID=Clang \
>     -DCMAKE_BUILD_TYPE=Debug \
>     -DCMAKE_CXX_COMPILER=$MYCLANG++ \
>     -DCMAKE_C_COMPILER=$MYCLANG \
>     -DLLVM_TARGETS_TO_BUILD="X86;AArch64" \
>     -DLLVM_DEFAULT_TARGET_TRIPLE=x86_64-redhat-linux-gnu \
>     -DLLVM_ENABLE_ASSERTIONS=ON \
>     -DLLVM_ENABLE_LLD=ON \
>     -DLLVM_ENABLE_PROJECTS="clang;lld;bolt" \
>     -DCMAKE_EXE_LINKER_FLAGS="-L /usr/lib64" \
>     -DLLVM_ENABLE_EXPENSIVE_CHECKS=ON \
>     ../llvm
> 
>      ninja check-all
> ```
> Is something else missing?
Looks OK to me. I'd make sure the code is rebuilt from scratch and `_GLIBCXX_DEBUG` is defined, then look into why libc++ doesn't do the checks. I'm attaching a backtrace of one of the failures, in case it may be of any help.

That regardless, I think the nature of the problem is very clear, which is that when the Begin1/2 and End1/2 iterators are singular, they should not be used to form any ranges. Once we have a fix, I'd be happy to test it for you.

```
[ RUN      ] CodeLayout.ThreeFunctions
/usr/include/c++/11/debug/vector:617:
In function:
    std::debug::vector<_Tp, _Allocator>::iterator std::debug::vector<_Tp, 
    _Allocator>::insert(std::debug::vector<_Tp, _Allocator>::const_iterator, 
    _InputIterator, _InputIterator) [with _InputIterator = 
    gnu_debug::_Safe_iterator<gnu_cxx::normal_iterator<{anonymous}::NodeT* 
    const*, std::vector<{anonymous}::NodeT*, 
    std::allocator<{anonymous}::NodeT*> > >, std::
    debug::vector<{anonymous}::NodeT*>, std::random_access_iterator_tag>; 
    <template-parameter-2-2> = void; _Tp = {anonymous}::NodeT*; _Allocator = 
    std::allocator<{anonymous}::NodeT*>; std::debug::vector<_Tp, 
    _Allocator>::iterator = std::
    debug::vector<{anonymous}::NodeT*>::iterator; std::debug::vector<_Tp, 
    _Allocator>::const_iterator = std::
    debug::vector<{anonymous}::NodeT*>::const_iterator]

Error: function requires a valid iterator range [first, last).

Objects involved in the operation:
    iterator "first" @ 0x7ffe7dbd1c40 {
      state = singular;
    }
    iterator "last" @ 0x7ffe7dbd1c70 {
      state = singular;
    }
 #0 0x00007f5c10bc3b02 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /home/kosarev/labs/llvm-project/llvm/lib/Support/Unix/Signals.inc:723:22
 #1 0x00007f5c10bc3f1e PrintStackTraceSignalHandler(void*) /home/kosarev/labs/llvm-project/llvm/lib/Support/Unix/Signals.inc:798:1
 #2 0x00007f5c10bc1363 llvm::sys::RunSignalHandlers() /home/kosarev/labs/llvm-project/llvm/lib/Support/Signals.cpp:105:20
 #3 0x00007f5c10bc339a SignalHandler(int) /home/kosarev/labs/llvm-project/llvm/lib/Support/Unix/Signals.inc:413:1
 #4 0x00007f5c10447520 (/lib/x86_64-linux-gnu/libc.so.6+0x42520)
 #5 0x00007f5c1049b9fc __pthread_kill_implementation ./nptl/./nptl/pthread_kill.c:44:76
 #6 0x00007f5c1049b9fc __pthread_kill_internal ./nptl/./nptl/pthread_kill.c:78:10
 #7 0x00007f5c1049b9fc pthread_kill ./nptl/./nptl/pthread_kill.c:89:10
 #8 0x00007f5c10447476 gsignal ./signal/../sysdeps/posix/raise.c:27:6
 #9 0x00007f5c1042d7f3 abort ./stdlib/./stdlib/abort.c:81:7
#10 0x00007f5c106d21fb std::__throw_bad_exception() (/lib/x86_64-linux-gnu/libstdc++.so.6+0xa51fb)
#11 0x00007f5c12dd8740 __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<(anonymous namespace)::NodeT**, std::__cxx1998::vector<(anonymous namespace)::NodeT*, std::allocator<(anonymous namespace)::NodeT*> > >, std::__debug::vector<(anonymous namespace)::NodeT*, std::allocator<(anonymous namespace)::NodeT*> >, std::random_access_iterator_tag> std::__debug::vector<(anonymous namespace)::NodeT*, std::allocator<(anonymous namespace)::NodeT*> >::insert<__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<(anonymous namespace)::NodeT* const*, std::__cxx1998::vector<(anonymous namespace)::NodeT*, std::allocator<(anonymous namespace)::NodeT*> > >, std::__debug::vector<(anonymous namespace)::NodeT*, std::allocator<(anonymous namespace)::NodeT*> >, std::random_access_iterator_tag>, void>(__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<(anonymous namespace)::NodeT* const*, std::__cxx1998::vector<(anonymous namespace)::NodeT*, std::allocator<(anonymous namespace)::NodeT*> > >, std::__debug::vector<(anonymous namespace)::NodeT*, std::allocator<(anonymous namespace)::NodeT*> >, std::random_access_iterator_tag>, __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<(anonymous namespace)::NodeT* const*, std::__cxx1998::vector<(anonymous namespace)::NodeT*, std::allocator<(anonymous namespace)::NodeT*> > >, std::__debug::vector<(anonymous namespace)::NodeT*, std::allocator<(anonymous namespace)::NodeT*> >, std::random_access_iterator_tag>, __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<(anonymous namespace)::NodeT* const*, std::__cxx1998::vector<(anonymous namespace)::NodeT*, std::allocator<(anonymous namespace)::NodeT*> > >, std::__debug::vector<(anonymous namespace)::NodeT*, std::allocator<(anonymous namespace)::NodeT*> >, std::random_access_iterator_tag>) /usr/include/c++/11/debug/vector:617:4
#12 0x00007f5c12dcee70 (anonymous namespace)::MergedChain::getNodes() const /home/kosarev/labs/llvm-project/llvm/lib/Transforms/Utils/CodeLayout.cpp:504:18
#13 0x00007f5c12dd5141 (anonymous namespace)::CDSortImpl::mergeChains((anonymous namespace)::ChainT*, (anonymous namespace)::ChainT*, unsigned long, (anonymous namespace)::MergeTypeT) /home/kosarev/labs/llvm-project/llvm/lib/Transforms/Utils/CodeLayout.cpp:1288:16
#14 0x00007f5c12dd4211 (anonymous namespace)::CDSortImpl::mergeChainPairs() /home/kosarev/labs/llvm-project/llvm/lib/Transforms/Utils/CodeLayout.cpp:1144:50
#15 0x00007f5c12dd302c (anonymous namespace)::CDSortImpl::run() /home/kosarev/labs/llvm-project/llvm/lib/Transforms/Utils/CodeLayout.cpp:1008:5
#16 0x00007f5c12dd615c llvm::codelayout::computeCacheDirectedLayout(llvm::codelayout::CDSortConfig const&, llvm::ArrayRef<unsigned long>, llvm::ArrayRef<unsigned long>, llvm::ArrayRef<llvm::codelayout::EdgeCount>, llvm::ArrayRef<unsigned long>) /home/kosarev/labs/llvm-project/llvm/lib/Transforms/Utils/CodeLayout.cpp:1435:3
#17 0x00007f5c12dd6333 llvm::codelayout::computeCacheDirectedLayout(llvm::ArrayRef<unsigned long>, llvm::ArrayRef<unsigned long>, llvm::ArrayRef<llvm::codelayout::EdgeCount>, llvm::ArrayRef<unsigned long>) /home/kosarev/labs/llvm-project/llvm/lib/Transforms/Utils/CodeLayout.cpp:1453:48
#18 0x00005638fc0abb97 (anonymous namespace)::CodeLayout_ThreeFunctions_Test::TestBody() /home/kosarev/labs/llvm-project/llvm/unittests/Transforms/Utils/CodeLayoutTest.cpp:18:78
#19 0x00007f5c13e2ef1a void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/kosarev/labs/llvm-project/third-party/unittest/googletest/src/gtest.cc:2612:28
#20 0x00007f5c13e1f20f void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/kosarev/labs/llvm-project/third-party/unittest/googletest/src/gtest.cc:2667:75
#21 0x00007f5c13df2bca testing::Test::Run() /home/kosarev/labs/llvm-project/third-party/unittest/googletest/src/gtest.cc:2694:30
#22 0x00007f5c13df350b testing::TestInfo::Run() /home/kosarev/labs/llvm-project/third-party/unittest/googletest/src/gtest.cc:2839:3
#23 0x00007f5c13df3eea testing::TestSuite::Run() /home/kosarev/labs/llvm-project/third-party/unittest/googletest/src/gtest.cc:3017:35
#24 0x00007f5c13e01306 testing::internal::UnitTestImpl::RunAllTests() /home/kosarev/labs/llvm-project/third-party/unittest/googletest/src/gtest.cc:5921:41
#25 0x00007f5c13e315fd bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /home/kosarev/labs/llvm-project/third-party/unittest/googletest/src/gtest.cc:2614:1
#26 0x00007f5c13e20b7a bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /home/kosarev/labs/llvm-project/third-party/unittest/googletest/src/gtest.cc:2667:75
#27 0x00007f5c13dffc25 testing::UnitTest::Run() /home/kosarev/labs/llvm-project/third-party/unittest/googletest/src/gtest.cc:5487:14
#28 0x00007f5c13efde7f RUN_ALL_TESTS() /home/kosarev/labs/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h:2317:80
#29 0x00007f5c13efdd61 main /home/kosarev/labs/llvm-project/third-party/unittest/UnitTestMain/TestMain.cpp:55:24
#30 0x00007f5c1042ed90 __libc_start_call_main ./csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#31 0x00007f5c1042ee40 call_init ./csu/../csu/libc-start.c:128:20
#32 0x00007f5c1042ee40 __libc_start_main ./csu/../csu/libc-start.c:379:5
#33 0x00005638fc0423e5 _start (/home/kosarev/labs/llvm-project/build/debug+expensive_checks/unittests/Transforms/Utils/./UtilsTests+0x1f3e5)
```

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113424/new/

https://reviews.llvm.org/D113424