Page MenuHomePhabricator

davezarzycki (David Zarzycki)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 28 2017, 11:01 AM (252 w, 3 d)

Recent Activity

Mar 25 2022

davezarzycki added a comment to D122482: [X86] combineADC - fold ADC(C1,C2,Carry) -> ADC(0,C1+C2,Carry).

Okay. I'll try to remember to create a followup bug after this lands. I'm sure I'm naive about it, but it seems to me that after register allocation, MOV $0, <reg> instructions should move earlier (when possible) in the basic block to get out of the way of EFLAG dependencies and allow the MOV $0, <reg> to XOR <reg>, <reg> optimization, but that's just me hand waving.

Mar 25 2022, 10:55 AM · Restricted Project, Restricted Project
davezarzycki added a comment to D122482: [X86] combineADC - fold ADC(C1,C2,Carry) -> ADC(0,C1+C2,Carry).

I was just surprised to see that the last four lines didn't swap the BT and the zeroing of EDX. Maybe this is a separate problem. This seems like the natural output of the last four lines of the example above:

Mar 25 2022, 9:44 AM · Restricted Project, Restricted Project
davezarzycki added a comment to D122482: [X86] combineADC - fold ADC(C1,C2,Carry) -> ADC(0,C1+C2,Carry).

This is a great improvement but a scenario is still missing. For example, from a build of LLVM with this change applied (i.e. self hosted):

Mar 25 2022, 9:03 AM · Restricted Project, Restricted Project

Oct 5 2021

davezarzycki added a comment to rG79bf032fe103: [lldb testing] Avoid subtle terminfo behavioral differences.

5bc32ad08d9a25b1a4fc4fe7daa4056d1d1ef67f

Oct 5 2021, 10:41 AM
davezarzycki committed rG5bc32ad08d9a: [lldb testing] NFC: run through clang-format (authored by davezarzycki).
[lldb testing] NFC: run through clang-format
Oct 5 2021, 10:41 AM
davezarzycki added a comment to D110962: [lldb] Add unit tests for Terminal API.

If you want to test more, please let me know and I can test them on my Fedora box.

Oct 5 2021, 10:36 AM · Restricted Project
davezarzycki added a comment to D110962: [lldb] Add unit tests for Terminal API.

79bf032fe103333546fd1d6e14c5ac8905f25c2b

Oct 5 2021, 7:29 AM · Restricted Project
davezarzycki committed rG79bf032fe103: [lldb testing] Avoid subtle terminfo behavioral differences (authored by davezarzycki).
[lldb testing] Avoid subtle terminfo behavioral differences
Oct 5 2021, 7:29 AM
davezarzycki added a comment to D110962: [lldb] Add unit tests for Terminal API.

"the buildbot"? There are many. I'd be surprised if this weren't failing on at least one of them. It doesn't seem like subtle terminfo behavior is essential to this test. Can we please trim down the adjustments to only changing the speed? For example:

diff --git i/lldb/unittests/Host/posix/TerminalTest.cpp w/lldb/unittests/Host/posix/TerminalTest.cpp
index ecdb5480216439903b9fc12c39b3d47cb62f9134..e865d44bf6cb9085f07c11e06be34f33a7bd32b9 100644
--- i/lldb/unittests/Host/posix/TerminalTest.cpp
+++ w/lldb/unittests/Host/posix/TerminalTest.cpp
@@ -80,14 +80,8 @@ TEST_F(TerminalTest, SaveRestoreRAII) {
     terminfo = orig_terminfo;
Oct 5 2021, 7:15 AM · Restricted Project
davezarzycki added a comment to D110962: [lldb] Add unit tests for Terminal API.

The FD in question is 3. Here is the output from lsof: HostTests 80108 dave 3u CHR 5,2 0t0 231 /dev/ptmx

Oct 5 2021, 5:01 AM · Restricted Project
davezarzycki added a comment to D110962: [lldb] Add unit tests for Terminal API.

This is failing on my Fedora 34 (x86-64) box. Is this expected?

FAIL: lldb-unit :: Host/./HostTests/TerminalTest.SaveRestoreRAII (78548 of 79402)
******************** TEST 'lldb-unit :: Host/./HostTests/TerminalTest.SaveRestoreRAII' FAILED ********************
Script:
--
/tmp/_update_lc/r/tools/lldb/unittests/Host/./HostTests --gtest_filter=TerminalTest.SaveRestoreRAII
--
Note: Google Test filter = TerminalTest.SaveRestoreRAII
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from TerminalTest
[ RUN      ] TerminalTest.SaveRestoreRAII
/home/dave/ro_s/lp/lldb/unittests/Host/posix/TerminalTest.cpp:93: Failure
Expected equality of these values:
  tcsetattr(m_pty.GetPrimaryFileDescriptor(), 0, &terminfo)
    Which is: -1
  0
[  FAILED  ] TerminalTest.SaveRestoreRAII (4 ms)
[----------] 1 test from TerminalTest (4 ms total)
Oct 5 2021, 4:05 AM · Restricted Project

Sep 18 2021

davezarzycki added a comment to D91944: OpenMP 5.0 metadirective.

Also, when this lands again, please ensure that all the tests don't accidentally dump build artifacts into the source directory. Specifically, the following test was missing -o -: clang/test/OpenMP/metadirective_messages.cpp

Sep 18 2021, 6:33 AM · Restricted Project, Restricted Project, Restricted Project

Jul 1 2021

davezarzycki accepted D105208: [lit] Extend --xfail/LIT_XFAIL to take full test name.
Jul 1 2021, 3:32 AM · Restricted Project
davezarzycki added a comment to D105208: [lit] Extend --xfail/LIT_XFAIL to take full test name.

LGTM too. And I agree that substring matching would be undesirable.

Jul 1 2021, 3:32 AM · Restricted Project

Jun 30 2021

davezarzycki added a comment to D105095: [Coroutine] Add statistics for the number of elided coroutine.

The -stats command line flag requires asserts, so either the test needs REQUIRES: asserts or the stats testing needs to be broken into a separate test.

Jun 30 2021, 8:26 AM · Restricted Project

Jun 7 2021

davezarzycki added a comment to D103820: [X86] Prefer vpmovq2m over vpternlogd + vpcmpgtq.

@davezarzycki What's going on the way phab is reporting the path in Revision Contents pane? llvm/lib/Target/X86 -> i/llvm/lib/Target/X86 ?!?

Jun 7 2021, 12:50 PM · Restricted Project
davezarzycki added a comment to D103820: [X86] Prefer vpmovq2m over vpternlogd + vpcmpgtq.

I filed a bug about the IR not being canonicalized. Let's wait for that before proceeding on generating VPMOVQ2M:

Jun 7 2021, 11:19 AM · Restricted Project
davezarzycki added inline comments to D103820: [X86] Prefer vpmovq2m over vpternlogd + vpcmpgtq.
Jun 7 2021, 10:29 AM · Restricted Project
davezarzycki added a comment to D103820: [X86] Prefer vpmovq2m over vpternlogd + vpcmpgtq.

Actually, wait. Something weird is going on at the mid-level. These two functions should generate the same optimized IR, right?

typedef int V __attribute__((vector_size(64)));

V lt_zero_x_y(V mask, V x, V y) { return mask <  0 ? x : y; }
V ge_zero_y_x(V mask, V x, V y) { return mask >= 0 ? y : x; }

I'm not sure we canonicalize select operand order.

vpmovq2m only exists with avx512dq. Is this transform still useful with just avx512f?

Jun 7 2021, 10:14 AM · Restricted Project
davezarzycki added a comment to D103820: [X86] Prefer vpmovq2m over vpternlogd + vpcmpgtq.

Actually, wait. Something weird is going on at the mid-level. These two functions should generate the same optimized IR, right?

Jun 7 2021, 9:46 AM · Restricted Project
davezarzycki updated the diff for D103820: [X86] Prefer vpmovq2m over vpternlogd + vpcmpgtq.
Jun 7 2021, 9:17 AM · Restricted Project
davezarzycki added a comment to D103820: [X86] Prefer vpmovq2m over vpternlogd + vpcmpgtq.

I'm surprised that this didn't break existing tests. Where is the right place to update the test suite?

Jun 7 2021, 8:27 AM · Restricted Project
davezarzycki requested review of D103820: [X86] Prefer vpmovq2m over vpternlogd + vpcmpgtq.
Jun 7 2021, 8:26 AM · Restricted Project

May 28 2021

davezarzycki added a comment to D103128: [GlobalISel] Don't emit lost debug location remarks when legalizing tail calls.

https://bugs.llvm.org/show_bug.cgi?id=50516

May 28 2021, 3:33 AM · Restricted Project

May 27 2021

davezarzycki added a comment to D103275: Update musttail verification to check all swiftasync->swiftasync tail calls..

LGTM but I don't feel ownership over this code.

May 27 2021, 12:05 PM · Restricted Project
davezarzycki added a comment to D103128: [GlobalISel] Don't emit lost debug location remarks when legalizing tail calls.

Sorry for the noise. I reran the bisect. The test in question is still failing, but it turns out to be failing unreliably so git-blame is fairly useless without a reliable test.

May 27 2021, 10:33 AM · Restricted Project
davezarzycki updated subscribers of D103128: [GlobalISel] Don't emit lost debug location remarks when legalizing tail calls.

I have an auto-bisecting multi-stage cron job that has identified this change as the source of a regression. Does the following seem plausible?

May 27 2021, 6:40 AM · Restricted Project

May 25 2021

davezarzycki added a comment to D101970: [X86FixupLEAs] Transform the sequence LEA/SUB to SUB/SUB.

I've verified that this patch no longer causes my multi-stage cron job to regress. Thanks!

May 25 2021, 12:38 PM · Restricted Project
davezarzycki added a comment to D101970: [X86FixupLEAs] Transform the sequence LEA/SUB to SUB/SUB.

Actually, it is more complicated, and you're right that it also can involve overflow. I'm not sure if this is a trivial optimization when dependent comparisons are involved.

May 25 2021, 5:09 AM · Restricted Project
davezarzycki added a comment to D101970: [X86FixupLEAs] Transform the sequence LEA/SUB to SUB/SUB.

I can check it, but I'm a bit concerned about the comment and what it implies. The problem is NOT overflow, but that comparison operations need to be redistributed during this optimization (or dependent comparisons need to disable the optimization). So in pseudocode, the original code that miscompiled looks like this:

May 25 2021, 4:55 AM · Restricted Project

May 19 2021

davezarzycki added a comment to D101970: [X86FixupLEAs] Transform the sequence LEA/SUB to SUB/SUB.

Thanks! Also, I feel fairly confident about this. My auto-bisecting cron job is really paranoid. For example, it always does a multi-stage test to verify that the commit *before* the commit blamed by git-bisect actually works. That's what I was waiting for. Also, I then verified by trying to move one commit ahead in the history to this commit to get an A/B test and I then confirmed that this is the regression. Thanks again for reverting this.

May 19 2021, 7:11 AM · Restricted Project
davezarzycki added a comment to D101970: [X86FixupLEAs] Transform the sequence LEA/SUB to SUB/SUB.

Verified. Second stage miscompile. If it matters (and it sometimes does), my first stage is built without asserts.

May 19 2021, 5:40 AM · Restricted Project
davezarzycki added a comment to D101970: [X86FixupLEAs] Transform the sequence LEA/SUB to SUB/SUB.

Lots of crashes like this:

FAIL: Clang-Unit :: Format/./FormatTests/FormatTest.FormatsCompactNamespaces (23265 of 76779)
******************** TEST 'Clang-Unit :: Format/./FormatTests/FormatTest.FormatsCompactNamespaces' FAILED ********************
Note: Google Test filter = FormatTest.FormatsCompactNamespaces
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from FormatTest
[ RUN      ] FormatTest.FormatsCompactNamespaces
FormatTests: /home/dave/ro_s/lp/clang/lib/Basic/SourceManager.cpp:865: clang::FileID clang::SourceManager::getFileIDLoaded(unsigned int) const: Assertion `0 && "Invalid SLocOffset or bad function choice"' failed.
 #0 0x0000000000604313 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/tmp/_update_lc/t/tools/clang/unittests/Format/./FormatTests+0x604313)
 #1 0x0000000000602962 llvm::sys::RunSignalHandlers() (/tmp/_update_lc/t/tools/clang/unittests/Format/./FormatTests+0x602962)
 #2 0x0000000000604d8a SignalHandler(int) Signals.cpp:0:0
 #3 0x00007ffff7fa4a20 (/lib64/libpthread.so.0+0x13a20)
 #4 0x00007ffff7b392a2 raise (/lib64/libc.so.6+0x3d2a2)
 #5 0x00007ffff7b228a4 abort (/lib64/libc.so.6+0x268a4)
 #6 0x00007ffff7b22789 (/lib64/libc.so.6+0x26789)
 #7 0x00007ffff7b31a16 (/lib64/libc.so.6+0x35a16)
 #8 0x000000000064922c clang::SourceManager::getFileIDLoaded(unsigned int) const (/tmp/_update_lc/t/tools/clang/unittests/Format/./FormatTests+0x64922c)
 #9 0x00000000003e734c clang::SourceManager::getDecomposedLoc(clang::SourceLocation) const CleanupTest.cpp:0:0
#10 0x000000000064b231 clang::SourceManager::isBeforeInTranslationUnit(clang::SourceLocation, clang::SourceLocation) const (/tmp/_update_lc/t/tools/clang/unittests/Format/./FormatTests+0x64b231)
#11 0x000000000066c05e clang::format::AffectedRangeManager::computeAffectedLines(llvm::SmallVectorImpl<clang::format::AnnotatedLine*>&) (/tmp/_update_lc/t/tools/clang/unittests/Format/./FormatTests+0x66c05e)
#12 0x00000000006ae393 clang::format::UsingDeclarationsSorter::analyze(clang::format::TokenAnnotator&, llvm::SmallVectorImpl<clang::format::AnnotatedLine*>&, clang::format::FormatTokenLexer&) (/tmp/_update_lc/t/tools/clang/unittests/Format/./FormatTests+0x6ae393)
#13 0x0000000000690d1a clang::format::TokenAnalyzer::process() (/tmp/_update_lc/t/tools/clang/unittests/Format/./FormatTests+0x690d1a)
#14 0x0000000000665643 std::__1::__function::__func<clang::format::internal::reformat(clang::format::FormatStyle const&, llvm::StringRef, llvm::ArrayRef<clang::tooling::Range>, unsigned int, unsigned int, unsigned int, llvm::StringRef, clang::format::FormattingAttemptStatus*)::$_4, std::__1::allocator<clang::format::internal::reformat(clang::format::FormatStyle const&, llvm::StringRef, llvm::ArrayRef<clang::tooling::Range>, unsigned int, unsigned int, unsigned int, llvm::StringRef, clang::format::FormattingAttemptStatus*)::$_4>, std::__1::pair<clang::tooling::Replacements, unsigned int> (clang::format::Environment const&)>::operator()(clang::format::Environment const&) Format.cpp:0:0
#15 0x0000000000655d6e clang::format::internal::reformat(clang::format::FormatStyle const&, llvm::StringRef, llvm::ArrayRef<clang::tooling::Range>, unsigned int, unsigned int, unsigned int, llvm::StringRef, clang::format::FormattingAttemptStatus*) (/tmp/_update_lc/t/tools/clang/unittests/Format/./FormatTests+0x655d6e)
#16 0x000000000065640f clang::format::reformat(clang::format::FormatStyle const&, llvm::StringRef, llvm::ArrayRef<clang::tooling::Range>, llvm::StringRef, clang::format::FormattingAttemptStatus*) (/tmp/_update_lc/t/tools/clang/unittests/Format/./FormatTests+0x65640f)
#17 0x00000000003ef27f clang::format::(anonymous namespace)::FormatTest::format(llvm::StringRef, clang::format::FormatStyle const&, clang::format::(anonymous namespace)::FormatTest::StatusCheck) FormatTest.cpp:0:0
#18 0x0000000000407eed clang::format::(anonymous namespace)::FormatTest_FormatsCompactNamespaces_Test::TestBody() FormatTest.cpp:0:0
#19 0x000000000060cc53 testing::Test::Run() (/tmp/_update_lc/t/tools/clang/unittests/Format/./FormatTests+0x60cc53)
#20 0x000000000060dc16 testing::TestInfo::Run() (/tmp/_update_lc/t/tools/clang/unittests/Format/./FormatTests+0x60dc16)
#21 0x000000000060e470 testing::TestSuite::Run() (/tmp/_update_lc/t/tools/clang/unittests/Format/./FormatTests+0x60e470)
#22 0x000000000061bb23 testing::internal::UnitTestImpl::RunAllTests() (/tmp/_update_lc/t/tools/clang/unittests/Format/./FormatTests+0x61bb23)
#23 0x000000000061b59d testing::UnitTest::Run() (/tmp/_update_lc/t/tools/clang/unittests/Format/./FormatTests+0x61b59d)
#24 0x000000000060555b main (/tmp/_update_lc/t/tools/clang/unittests/Format/./FormatTests+0x60555b)
#25 0x00007ffff7b23b75 __libc_start_main (/lib64/libc.so.6+0x27b75)
#26 0x00000000003ddd8e _start (/tmp/_update_lc/t/tools/clang/unittests/Format/./FormatTests+0x3ddd8e)
May 19 2021, 4:41 AM · Restricted Project
davezarzycki added a comment to D101970: [X86FixupLEAs] Transform the sequence LEA/SUB to SUB/SUB.

Just a heads up, my auto-bisecting multi-stage cron job has identified this change as the source of a second stage regression in a bunch of clang unit tests. I'm about to verify by hand, but it'll take a while.

May 19 2021, 4:28 AM · Restricted Project

May 17 2021

davezarzycki added a comment to D100043: [lit] Fix compatibility with upstream gtest.

LGTM but I don't feel authoritative for the lit code.

May 17 2021, 4:28 AM · Restricted Project

May 6 2021

davezarzycki added a comment to D99949: [AMDGPU][OpenMP] Add amdgpu-arch tool to list AMD GPUs installed.

Yes, Fedora 34 (x86-64).

May 6 2021, 2:38 PM · Restricted Project

May 4 2021

davezarzycki committed rG05146fe51710: [llvm] Unbreak no-assertion testing (authored by davezarzycki).
[llvm] Unbreak no-assertion testing
May 4 2021, 3:06 AM

Apr 29 2021

davezarzycki committed rG3eb2be67b997: Unbreak no-asserts testing (authored by davezarzycki).
Unbreak no-asserts testing
Apr 29 2021, 7:19 AM

Apr 27 2021

davezarzycki committed rG646b007d1128: [llvm] Unbreak no-asserts testing after 18839be9c5c8b9f882dd241769784035b082d4e1 (authored by davezarzycki).
[llvm] Unbreak no-asserts testing after 18839be9c5c8b9f882dd241769784035b082d4e1
Apr 27 2021, 2:47 AM

Apr 23 2021

davezarzycki added a comment to D101180: Mark type test intrinsics as speculatable to fix inline cost.

I have an auto bisecting cron job that has twice identified this change as the source of a regression when building release (no asserts). Is the following expected or can we get a quick fix?

Apr 23 2021, 3:18 PM · Restricted Project
davezarzycki added a comment to D99949: [AMDGPU][OpenMP] Add amdgpu-arch tool to list AMD GPUs installed.

It's still failing but I can disable HSA on this machine for now. FYI --

FAILED: tools/clang/tools/amdgpu-arch/CMakeFiles/amdgpu-arch.dir/AMDGPUArch.cpp.o
/p/tllvm/bin/clang++ -DGTEST_HAS_RTTI=0 -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/clang/tools/amdgpu-arch -I/home/dave/ro_s/lp/clang/tools/amdgpu-arch -I/home/dave/ro_s/lp/clang/include -Itools/clang/include -Iinclude -I/home/dave/ro_s/lp/llvm/include -Werror=switch -Wno-deprecated-copy -stdlib=libc++ -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O2 -DNDEBUG  -march=skylake -fno-vectorize -fno-slp-vectorize -fno-tree-slp-vectorize -fno-exceptions -fno-rtti -std=c++14 -MD -MT tools/clang/tools/amdgpu-arch/CMakeFiles/amdgpu-arch.dir/AMDGPUArch.cpp.o -MF tools/clang/tools/amdgpu-arch/CMakeFiles/amdgpu-arch.dir/AMDGPUArch.cpp.o.d -o tools/clang/tools/amdgpu-arch/CMakeFiles/amdgpu-arch.dir/AMDGPUArch.cpp.o -c /home/dave/ro_s/lp/clang/tools/amdgpu-arch/AMDGPUArch.cpp
/home/dave/ro_s/lp/clang/tools/amdgpu-arch/AMDGPUArch.cpp:14:10: fatal error: 'hsa.h' file not found
#include <hsa.h>
         ^~~~~~~
1 error generated.
Apr 23 2021, 6:33 AM · Restricted Project

Apr 22 2021

davezarzycki added a comment to D99949: [AMDGPU][OpenMP] Add amdgpu-arch tool to list AMD GPUs installed.

I removed all HSA/ROCM via the package manager and everything is fine now. Have you considered using __has_include() instead of CMake? It seems to be supported by all contemporary compilers, even MSVC.

Apr 22 2021, 3:20 PM · Restricted Project
davezarzycki added a comment to D99949: [AMDGPU][OpenMP] Add amdgpu-arch tool to list AMD GPUs installed.

It's not obvious to me why any of those stages would get a different result for the search for rocr. Do you do things with chroot/jails to ensure isolation for some of them?

Apr 22 2021, 12:11 PM · Restricted Project
davezarzycki added a comment to D99949: [AMDGPU][OpenMP] Add amdgpu-arch tool to list AMD GPUs installed.

That's interesting. The various permutations I have on disk are also all path/include/hsa/hsa.h. The existing in tree use of hsa.h is the amdgpu plugin, which uses #include "hsa.h" and #include <hsa.h>, which seems unlikely to be correct.

I'm going to patch this on the fly to hsa/hsa.h as that looks very likely to be the fix.

edit: or not, the include path cmake summons on my local system is -isystem $HOME/llvm-install/include/hsa, so hsa/hsa doesn't work.

Tiresome. Will revert this for now.

Apr 22 2021, 11:41 AM · Restricted Project
davezarzycki added a comment to D99949: [AMDGPU][OpenMP] Add amdgpu-arch tool to list AMD GPUs installed.

And if it helps:

Apr 22 2021, 11:16 AM · Restricted Project
davezarzycki added a comment to D99949: [AMDGPU][OpenMP] Add amdgpu-arch tool to list AMD GPUs installed.

This change broke multi-stage builds (even if AMDGPU is disabled as a target). The problem is that clang/tools/amdgpu-arch/AMDGPUArch.cpp can't find hsa.h. Is there a quick fix?

That should not be the case. The cmake for building AMDGPUArch.cpp is guarded by

find_package(hsa-runtime64 QUIET 1.2.0 HINTS ${CMAKE_INSTALL_PREFIX} PATHS /opt/rocm)
if (NOT ${hsa-runtime64_FOUND})
    return

The intent is for this to be built only when hsa-runtime64 is found on disk, at which point hsa.h is meant to be found within that package via cmake magic. Perhaps we need to explicitly specify the include directory, though we do not do so in the openmp plugin this was copied from, and I do not have hsa.h on any global search path.

Little harm will be done by reverting this (again...) and reapplying, but it would be really helpful if you can share some more information about the build config that is failing.

Apr 22 2021, 11:14 AM · Restricted Project
davezarzycki added a comment to D99949: [AMDGPU][OpenMP] Add amdgpu-arch tool to list AMD GPUs installed.

This change broke multi-stage builds (even if AMDGPU is disabled as a target). The problem is that clang/tools/amdgpu-arch/AMDGPUArch.cpp can't find hsa.h. Is there a quick fix?

Apr 22 2021, 10:21 AM · Restricted Project
davezarzycki added a comment to D100073: [libcxx][iterator] adds `std::indirectly_readable` and `std::indirectly_writable`.

I'm the third person to notice the same problems, but this time on Fedora 34 (x86-64) so I reverted the change.

Apr 22 2021, 6:52 AM · Restricted Project
davezarzycki added a reverting change for rG04733181b513: [libcxx][iterator] adds `std::indirectly_readable` and `std…: rGa9f11cc0d965: Revert "[libcxx][iterator] adds `std::indirectly_readable` and `std….
Apr 22 2021, 6:50 AM
davezarzycki committed rGa9f11cc0d965: Revert "[libcxx][iterator] adds `std::indirectly_readable` and `std… (authored by davezarzycki).
Revert "[libcxx][iterator] adds `std::indirectly_readable` and `std…
Apr 22 2021, 6:50 AM
davezarzycki added a reverting change for D100073: [libcxx][iterator] adds `std::indirectly_readable` and `std::indirectly_writable`: rGa9f11cc0d965: Revert "[libcxx][iterator] adds `std::indirectly_readable` and `std….
Apr 22 2021, 6:50 AM · Restricted Project
davezarzycki added a comment to D100876: [libc++] [test] Remove epicyclic workarounds for vector/span; use T[] or std::array.

My mistake. The bisect tripped over a unreliable test. The real problem was 04733181b5136e2b3df2b37c6bdd9e25f8afecd0 and people are already piling onto that as being broken.

Apr 22 2021, 6:38 AM · Restricted Project
davezarzycki added a comment to D100876: [libc++] [test] Remove epicyclic workarounds for vector/span; use T[] or std::array.

My automatically bisecting cron job identified this commit as the source of a regression on Fedora 34 (x86-64). It seems like a few dozen libc++ tests are now failing with errors like this:

Apr 22 2021, 5:12 AM · Restricted Project

Apr 15 2021

davezarzycki added a comment to D98179: [lit] Sort test start times based on prior test timing data.

Something related to the time recording seems to fail intermittently on buildbots: https://lab.llvm.org/buildbot#builders/93/builds/2697

I'll cut to the fix here: @davezarzycki is there any reason why the timing data is stored in a CSV format, and not JSON?
The fix is to switch to latter.

Apr 15 2021, 7:28 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Apr 12 2021

davezarzycki added a comment to D91093: [X86][PPC] Tune SelectionDAG CTPOP emulation via TLI hook.

Sorry, I got distracted. If it were up to me, I'd prefer tuning numbers that are, without checking, unambiguously better than the current code gen. In particular the code gen for KNL is awful. Furthermore, I think that fine tuning this further has diminishing returns for various reasons. Are people okay with that?

Apr 12 2021, 5:25 AM · Restricted Project

Apr 6 2021

davezarzycki added a comment to D98884: [IR] Ignore bitcasts of function pointers which are only used as callees in callbase instruction.

These failures were not caught by LLVM bots which run on a patch. Can I do a trial run of the bots and run all tests that would run when the patch lands?

Apr 6 2021, 12:54 PM · Restricted Project
davezarzycki added a comment to D98884: [IR] Ignore bitcasts of function pointers which are only used as callees in callbase instruction.

I have an auto-bisecting cron job that has identified this change as a second-stage regression on Fedora 34 (x86-64). Basically llvm-tblgen crashes reliably:

FAILED: lib/Target/X86/X86GenDAGISel.inc
cd /tmp/_update_lc/t && /tmp/_update_lc/t/bin/llvm-tblgen -gen-dag-isel -I /home/dave/ro_s/lp/llvm/lib/Target/X86 -I/tmp/_update_lc/t/include -I/home/dave/ro_s/lp/llvm/include -I /home/dave/ro_s/lp/llvm/lib/Target -omit-comments /home/dave/ro_s/lp/llvm/lib/Target/X86/X86.td --write-if-changed -o lib/Target/X86/X86GenDAGISel.inc -d lib/Target/X86/X86GenDAGISel.inc.d
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
Stack dump:
0.  Program arguments: /tmp/_update_lc/t/bin/llvm-tblgen -gen-dag-isel -I /home/dave/ro_s/lp/llvm/lib/Target/X86 -I/tmp/_update_lc/t/include -I/home/dave/ro_s/lp/llvm/include -I /home/dave/ro_s/lp/llvm/lib/Target -omit-comments /home/dave/ro_s/lp/llvm/lib/Target/X86/X86.td --write-if-changed -o lib/Target/X86/X86GenDAGISel.inc -d lib/Target/X86/X86GenDAGISel.inc.d
#0 0x000000000050b153 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/tmp/_update_lc/t/bin/llvm-tblgen+0x50b153)
#1 0x00000000005094b2 llvm::sys::RunSignalHandlers() (/tmp/_update_lc/t/bin/llvm-tblgen+0x5094b2)
#2 0x000000000050b4fa SignalHandler(int) Signals.cpp:0:0
#3 0x00007ffff7fa6a00 __restore_rt sigaction.c:0:0
#4 0x0000000000316a7d (/tmp/_update_lc/t/bin/llvm-tblgen+0x316a7d)
Apr 6 2021, 7:42 AM · Restricted Project
davezarzycki accepted D99858: [lit testing] Fix xfail-cl.py test worker count.

Good catch! Thanks

Apr 6 2021, 2:08 AM · Restricted Project

Mar 25 2021

davezarzycki added inline comments to D95713: [lldb/Plugins] Add ScriptedProcess Process Plugin.
Mar 25 2021, 3:20 PM · Restricted Project

Mar 24 2021

davezarzycki added a comment to D97913: [runtimes] Use add_lit_testsuite to register lit testsuites.

Why do you use these settings?

-DLIBCXX_ENABLE_SHARED=FALSE
-DLIBCXXABI_ENABLE_SHARED=FALSE
-DLIBUNWIND_ENABLE_SHARED=FALSE

What is the intent of your build job?

I try to ensure that building/testing works with PIC disabled, which (of course) requires disabling shared libraries.

@davezarzycki The problem here is that this change enabled the libc++ and libc++abi tests in your job, which were not being run before. I think there might be issues with the configuration you're using and this change only makes those issues apparent.

Mar 24 2021, 8:07 AM · Restricted Project, Restricted Project, Restricted Project
davezarzycki added a comment to D95713: [lldb/Plugins] Add ScriptedProcess Process Plugin.

The tests are failing because Dave's bot is running without enabled Python. The same is true for the Windows bot. Putting the plugin behind #ifdef LLDB_ENABLE_PYTHON should fix this.

Mar 24 2021, 7:53 AM · Restricted Project
davezarzycki added a comment to D95713: [lldb/Plugins] Add ScriptedProcess Process Plugin.

I've reverted this. If you need help debugging this, please let me know. Also, as a reminder, please see the attached build log from earlier in the conversation. Thanks!

Mar 24 2021, 4:10 AM · Restricted Project
davezarzycki added a reverting change for rGdd391e1ef762: [lldb/Plugins] Add ScriptedProcess Process Plugin: rG952bc6c92e21: Revert "[lldb/Plugins] Add ScriptedProcess Process Plugin".
Mar 24 2021, 4:08 AM
davezarzycki committed rG952bc6c92e21: Revert "[lldb/Plugins] Add ScriptedProcess Process Plugin" (authored by davezarzycki).
Revert "[lldb/Plugins] Add ScriptedProcess Process Plugin"
Mar 24 2021, 4:08 AM
davezarzycki added a reverting change for D95713: [lldb/Plugins] Add ScriptedProcess Process Plugin: rG952bc6c92e21: Revert "[lldb/Plugins] Add ScriptedProcess Process Plugin".
Mar 24 2021, 4:08 AM · Restricted Project

Mar 23 2021

davezarzycki added a comment to D95713: [lldb/Plugins] Add ScriptedProcess Process Plugin.

An auto-bisecting cron job has identified this change as a regression on Fedora 33 (x86-64). Can we get a quick fix or revert this? Here is the build output:

Mar 23 2021, 6:52 PM · Restricted Project

Mar 22 2021

davezarzycki resigned from D99073: [lit] Reliable progress indicator and ETA.

Interesting. Thanks for looking into this and building on D98179. I'm sure that people will appreciate this change. :-)

Mar 22 2021, 6:43 AM · Restricted Project

Mar 20 2021

davezarzycki added a comment to rG243333ef3ec6: Revert "[Driver] Drop obsoleted Ubuntu 11.04 gcc detection".

Adding -stdlib=libstdc++ fixes it. Clang can default to libcxx so if the C++ standard library matters then the test needs to be explicit.

Mar 20 2021, 10:59 AM
davezarzycki added a comment to rG243333ef3ec6: Revert "[Driver] Drop obsoleted Ubuntu 11.04 gcc detection".

The very first RUN line seems to fail. Here is the output, sans FileCheck:

Mar 20 2021, 10:29 AM
davezarzycki added a comment to rG243333ef3ec6: Revert "[Driver] Drop obsoleted Ubuntu 11.04 gcc detection".

This seems a misfire. bed9933a461e7b3d0c8c5a8fa770aa1b49802660 had fixed the failure.

Mar 20 2021, 10:26 AM
davezarzycki added a comment to D98179: [lit] Sort test start times based on prior test timing data.

Why are timeouts important? Our use case is running Alive2 with the test suite. Alive2 is heavy machinery and runs into timeouts. Running the tests in roughly the same order every time is important to avoid timeout tests flipping to failed or vice-versa. Plus slow tests = tests that consume a lot of memory (in our scenario), so we can't bundle slow tests together.
Adding a --ignore-timing-data would be great, yes! But I still feel that sorting the list of failed tests is important for user experience. I diff these all the time.

That still sounds incredibly brittle. If there is any variety in test machine performance, then any and all attempts at sorting should be futile because the underlying hardware will perturb different timeouts. Is this not your experience? How do you reconcile hardware performance and configuration details (like SMT) with timeout settings?

Of course it's brittle :) Changing from a time-based setting to a ticks-based system is ongoing work, such that resource exhaustion becomes deterministic.
Nevertheless, on a same machine, we don't see many test flips. It's quite stable most of the times (just one test flip once in a while).

Mar 20 2021, 7:43 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
davezarzycki added a comment to D98179: [lit] Sort test start times based on prior test timing data.

I'm talking about sorting just the summary of failed tests, not the whole output. We need the whole -vv output, but that can be out of order.

Why are timeouts important? Our use case is running Alive2 with the test suite. Alive2 is heavy machinery and runs into timeouts. Running the tests in roughly the same order every time is important to avoid timeout tests flipping to failed or vice-versa. Plus slow tests = tests that consume a lot of memory (in our scenario), so we can't bundle slow tests together.
Adding a --ignore-timing-data would be great, yes! But I still feel that sorting the list of failed tests is important for user experience. I diff these all the time.

Mar 20 2021, 5:13 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
davezarzycki added a comment to D98179: [lit] Sort test start times based on prior test timing data.

I'm talking about sorting just the summary of failed tests, not the whole output. We need the whole -vv output, but that can be out of order.

Aha! +1 then, i think sorting summary should be both easy and nice to have.

Mar 20 2021, 4:55 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
davezarzycki committed rG5cbe2279f723: [lit] Sort testing summary output (authored by davezarzycki).
[lit] Sort testing summary output
Mar 20 2021, 4:55 AM
davezarzycki added a reverting change for rGbdf39e6b0ed4: [Driver] Drop obsoleted Ubuntu 11.04 gcc detection: rG243333ef3ec6: Revert "[Driver] Drop obsoleted Ubuntu 11.04 gcc detection".
Mar 20 2021, 4:31 AM
davezarzycki committed rG243333ef3ec6: Revert "[Driver] Drop obsoleted Ubuntu 11.04 gcc detection" (authored by davezarzycki).
Revert "[Driver] Drop obsoleted Ubuntu 11.04 gcc detection"
Mar 20 2021, 4:31 AM
davezarzycki added a comment to D98179: [lit] Sort test start times based on prior test timing data.

Can we revert to the previous behavior please? The current behavior is not user friendly. Thanks!

To clarify: you care about the order in the final summary, not the actual execution order, right? (the goal of this patch is the latter, if it changes the former this is just a side-effect I believe)

I don't love that the default test order was changed. For our use case that isn't good, but I can just delete the .lit_test_times.txt file and get the old fixed order. (the reason why having a fixed order is good is because of timeouts. Also, slow tests often consume more memory in our setting, so it's not good to run all the slow tests at the same time).

Mar 20 2021, 4:05 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
davezarzycki added a comment to D98179: [lit] Sort test start times based on prior test timing data.

No, I'm running lit and dumping into a file. sorting is not as easy as piping through sort, as I'm running with -vv (required).

Since tests run in parallel, don't you already have some non-determinism in the -vv output? Isn't it printing as test finish? (just curious, not pushing back on anything here).

Mar 20 2021, 3:45 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Mar 19 2021

davezarzycki added a comment to D97913: [runtimes] Use add_lit_testsuite to register lit testsuites.

Why do you use these settings?

-DLIBCXX_ENABLE_SHARED=FALSE
-DLIBCXXABI_ENABLE_SHARED=FALSE
-DLIBUNWIND_ENABLE_SHARED=FALSE

What is the intent of your build job?

Mar 19 2021, 4:19 AM · Restricted Project, Restricted Project, Restricted Project

Mar 17 2021

davezarzycki added inline comments to D98767: [lit] Harmonize test timing data between Unix and Windows.
Mar 17 2021, 12:12 PM · Restricted Project
davezarzycki committed rG2b20df2d798a: [lit] Harmonize test timing data between Unix and Windows (authored by davezarzycki).
[lit] Harmonize test timing data between Unix and Windows
Mar 17 2021, 4:43 AM
davezarzycki closed D98767: [lit] Harmonize test timing data between Unix and Windows.
Mar 17 2021, 4:43 AM · Restricted Project
davezarzycki requested review of D98767: [lit] Harmonize test timing data between Unix and Windows.
Mar 17 2021, 2:53 AM · Restricted Project

Mar 16 2021

davezarzycki added a comment to D98179: [lit] Sort test start times based on prior test timing data.

I've run out of time for today, but for the record, I think we should use slash as the canonical separator for the timing data file. The actual separator doesn't matter because these strings are never used as paths, just as keys into a dictionary. (Therefore any character would work.) This would let us harmonize the Unix and Windows timing data files.

Mar 16 2021, 10:37 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
davezarzycki added a comment to D98179: [lit] Sort test start times based on prior test timing data.

Ah yes, of course. I've marked that test as unavailable on Window for now: 61ca706461c5e1edc18526c9ddc3250fe074ed94

Mar 16 2021, 7:55 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
davezarzycki committed rG61ca706461c5: [lit testing] Mark reorder.py as unavailable on Windows (authored by davezarzycki).
[lit testing] Mark reorder.py as unavailable on Windows
Mar 16 2021, 7:54 AM
davezarzycki committed rG49d0e115d5df: [lit testing] Fix Windows reliability? (authored by davezarzycki).
[lit testing] Fix Windows reliability?
Mar 16 2021, 6:12 AM
davezarzycki added a comment to D98179: [lit] Sort test start times based on prior test timing data.

Try: 49d0e115d5df40aa89339f4ace7a8dee378c03bb

Mar 16 2021, 6:12 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
davezarzycki added a comment to D98179: [lit] Sort test start times based on prior test timing data.

Said differently, the Unix side seems to always run the lit tests from a fresh/clean setup, even for incremental builds. If Windows tests lit differently then ya, things will blow up.

Mar 16 2021, 6:06 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
davezarzycki added a comment to D98179: [lit] Sort test start times based on prior test timing data.

That implies a different testing environment. How different is the Windows testing environment compared to Unix? Do you specifically have a custom testing setup?

Mar 16 2021, 6:05 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
davezarzycki committed rG0fda5e844128: [llvm-exegesis testing] Workaround unreliable test (authored by davezarzycki).
[llvm-exegesis testing] Workaround unreliable test
Mar 16 2021, 5:00 AM
davezarzycki committed rG1d297f90649d: [lit] Sort test start times based on prior test timing data (authored by davezarzycki).
[lit] Sort test start times based on prior test timing data
Mar 16 2021, 2:23 AM
davezarzycki closed D98179: [lit] Sort test start times based on prior test timing data.
Mar 16 2021, 2:23 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
davezarzycki updated the diff for D98179: [lit] Sort test start times based on prior test timing data.

Removed the commented only-failures and fixed a nit before committing.

Mar 16 2021, 2:21 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Mar 15 2021

davezarzycki added a comment to D98179: [lit] Sort test start times based on prior test timing data.

Might somebody be willing to sign off on this change (this week or next)? I'd like to cherry-pick it to Swift's LLVM branch. Thanks for all the feedback so far.

Mar 15 2021, 7:55 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Mar 12 2021

davezarzycki updated the diff for D98179: [lit] Sort test start times based on prior test timing data.

I fixed the call to split as requested. Being unfamiliar with python, I didn't know that it supported keyword arguments / parameter labels. Thanks.

Mar 12 2021, 3:53 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
davezarzycki updated the diff for D98179: [lit] Sort test start times based on prior test timing data.

I've made all of the requested changes to date. Two notes:

Mar 12 2021, 3:16 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Mar 10 2021

davezarzycki removed 1 blocking reviewer(s) for D98179: [lit] Sort test start times based on prior test timing data: Restricted Project.

I'm downgrading libcxx as a reviewer to non-blocking because the latest patch no longer requires changes to their project.

Mar 10 2021, 7:30 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
davezarzycki updated the diff for D98179: [lit] Sort test start times based on prior test timing data.

I renamed the file variable as requested.

Mar 10 2021, 7:27 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
davezarzycki updated the diff for D98179: [lit] Sort test start times based on prior test timing data.

I believe I have addressed all of the feedback to date.

Mar 10 2021, 4:23 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project