|60,080 ms||x64 debian > ThreadSanitizer-x86_64.ThreadSanitizer-x86_64::restore_stack.cpp|
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -fsanitize=thread -Wall -m64 -msse4.2 -gline-tables-only -I/var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/tsan/../ -std=c++11 -I/var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/tsan/../ -O1 /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/tsan/restore_stack.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/tsan/X86_64Config/Output/restore_stack.cpp.tmp && not /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/tsan/X86_64Config/Output/restore_stack.cpp.tmp 2>&1 | FileCheck /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/tsan/restore_stack.cpp
|1,960 ms||x64 debian > libFuzzer.libFuzzer::set_cover_merge.test|
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -O2 -gline-tables-only -fsanitize=address,fuzzer -I/var/lib/buildkite-agent/builds/llvm-project/compiler-rt/lib/fuzzer -m64 /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/fuzzer/FullCoverageSetTest.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/fuzzer/X86_64DefaultLinuxConfig/Output/set_cover_merge.test.tmp-FullCoverageSetTest
ping :) @nikic can you please review this revision to make sure it is what you want in https://github.com/llvm/llvm-project/issues/54561#issuecomment-1079460876 ?
(I'm new to around this, so those comments are just my opinions.)
IMHO, duplicating SuccForValue map for each casts seems a little redundant. I like collecting incoming phi values in the front and checking correspondences to unify those maps. Then we can also early return if the values don't correspond.
Inverting can't be determined on these lines. If you do so, these comments look obsolete, and better to be modified
IMHO, this checking, which is done when creating maps, seems a bit redundant, it's better to hold conditions in some way.
Thanks for the feedback!
Can you elaborate your solution? Here are my worries:
Here we are going to find a operation that maps branch condition in to corresponding phi values. Not an operation that maps phi values back to branch condition.
I think I have to left "sext"/"zext"ed values here because we must know the result of the branch condition after "s/zext". That's different from truncating from PHI values. i.e. s/zext information is not recoverable. Truncating phi values to branch condition does not imply we can z/sext the branch condition back to "phi values".
Nice catch! I'd like to fixup this after all "functional change" reviews.
Sorry for my vague comments. The idea was unifying later inverting checking and these casting checking. Finding SExt or ZExt is similar to decide an inversion
I just thought it'll be neat to reverse the roll between Succ/Cond and Pred/Phi. Then we don't need to hold candidate values. But I feel this might be too detailed and optional, this requires the dominant condition reversed. (I just tried to write actually, but ugly wip https://github.com/llvm/llvm-project/compare/main...khei4:llvm-project:wip/khei4-sext