[MergeICmps] Adapt to non-eq comparisons

[Allen]

Summary

Fix the last runtime issue as some sequent comparisons need be spilted.
For the origin equal comparisons chain, the new spilted Icmp chain will
still be end with equal, while for the new not-equal comparisons chain,
the new spilted Icmp chain will still be end with equal, so should address
this carefully, see detail wih case partial_sequent_ne

[nikic]

Fix https://github.com/llvm/llvm-project/issues/59740.

This issue should remain open, because this only handles the ne case, not other predicates.

llvm/lib/Transforms/Scalar/MergeICmps.cpp

658

We should remember the correct predicate earlier and pass it in here, rather than rederiving it.

668

Builder.CreateICmp(Predicate, LhsLoad, RhsLoad)

llvm/test/Transforms/MergeICmps/X86/pr59740.ll

49

Missing negative test coverage, e.g. wrong constants in phi.

[Allen]

Address comment
1、Add a new negative test cmp_ne_incorrect_const
2、Add new parameter ICmpInst::Predicate Predicate for BCECmpChain、

simplify and mergeComparisons to pass the correct predicate.

3、Update with API Builder.CreateICmp

[courbet]

llvm/lib/Transforms/Scalar/MergeICmps.cpp

843

Predicate could be a member of BCECmpChain. It's conceptually is a property of the chain and this would make the code simpler.

[courbet]

llvm/lib/Transforms/Scalar/MergeICmps.cpp

352

Let's assert that ConstBase is an i1 here, because isOne() would be incorrect otherwise.

674

nit: this is no longer IsEqual

[Allen]

Address comments
1、Add member Predicate_ for BCECmpChain to make the code simpler
2、Add assert for ConstBase and Const to guard the type is i1
3、rename IsEqual to IsCmp

[Allen]

llvm/lib/Transforms/Scalar/MergeICmps.cpp

352

Apply your comment, thanks

843

Done, thanks

[courbet]

llvm/lib/Transforms/Scalar/MergeICmps.cpp

644

CmpResult, or ICmpValue ?

[Allen]

rename IsICmp to ICmpValue

[Allen]

llvm/lib/Transforms/Scalar/MergeICmps.cpp

644

Done, thanks

[vitalybuka]

Probably breaks https://lab.llvm.org/buildbot/#/builders/85/builds/13602
The rest on blame list looks irrelevant.

[Allen]

In D141188#4050141, @vitalybuka wrote:

Probably breaks https://lab.llvm.org/buildbot/#/builders/85/builds/13602
The rest on blame list looks irrelevant.

Thanks @vitalybuka for report, but how to reproduce the issue? I'll take a look.

[vitalybuka]

In D141188#4050177, @Allen wrote:

In D141188#4050141, @vitalybuka wrote:

Probably breaks https://lab.llvm.org/buildbot/#/builders/85/builds/13602
The rest on blame list looks irrelevant.

Thanks @vitalybuka for report, but how to reproduce the issue? I'll take a look.

precise reproducer is https://github.com/google/sanitizers/wiki/SanitizerBotReproduceBuild (you need to use buildbot_bootstrap_ubsan.sh script)

but maybe just this is enough:
cmake -GNinja -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=ON -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF -DCMAKE_C_COMPILER=<your patched clang>/bin/clang -DCMAKE_CXX_COMPILER=<your patched clang>/bin/clang++ -DLLVM_USE_LINKER=lld '-DLLVM_ENABLE_PROJECTS='\''clang;lld;clang-tools-extra;mlir'\''' -DCMAKE_BUILD_TYPE=Release -DLLVM_USE_SANITIZER=Undefined '-DCMAKE_C_FLAGS=-fsanitize=undefined' '-DCMAKE_CXX_FLAGS=-fsanitize=undefined' llvm-project/llvm

[Allen]

Thanks @vitalybuka very much, I now can build the project with following command.

• ~/scratch_dir/llvm-zorg/zorg/buildbot/builders/sanitizers/buildS/llvm-project(cbbcb10e084e) » sh ../../buildbot_bootstrap_ubsan.sh

• base test report before the MR

Slowest Tests:
--------------------------------------------------------------------------
155.50s: Clang :: Driver/fsanitize.c
150.15s: Clang :: Driver/arm-cortex-cpus-2.c
142.50s: Clang :: Driver/arm-cortex-cpus-1.c
112.45s: Clang :: Driver/linux-ld.c
106.76s: Clang :: OpenMP/target_defaultmap_codegen_01.cpp
102.41s: Clang :: OpenMP/target_update_codegen.cpp
94.30s: Clang :: Driver/cl-options.c
74.68s: Clang :: Driver/clang_f_opts.c
73.79s: Clang :: Preprocessor/arm-target-features.c
71.09s: Clang :: Driver/riscv-arch.c
70.59s: Clang :: Driver/debug-options.c
63.96s: Clang :: Driver/x86-target-features.c
62.39s: Clang :: Preprocessor/aarch64-target-features.c
60.43s: Clang :: Preprocessor/predefined-arch-macros.c
59.80s: Clang :: Driver/response-file.c
58.29s: Clang :: Driver/aarch64-fp16.c
55.18s: Clang :: Driver/cl-outputs.c
53.77s: Clang :: OpenMP/target_data_codegen.cpp
50.78s: Clang :: Driver/pic.c
49.10s: lit :: shtest-define.py
Tests Times:
--------------------------------------------------------------------------
[   Range   ] :: [               Percentage               ] :: [   Count   ]
--------------------------------------------------------------------------
[150s,160s) :: [                                        ] :: [    2/80132]
[140s,150s) :: [                                        ] :: [    1/80132]
[130s,140s) :: [                                        ] :: [    0/80132]
[120s,130s) :: [                                        ] :: [    0/80132]
[110s,120s) :: [                                        ] :: [    1/80132]
[100s,110s) :: [                                        ] :: [    2/80132]
[ 90s,100s) :: [                                        ] :: [    1/80132]
[ 80s, 90s) :: [                                        ] :: [    0/80132]
[ 70s, 80s) :: [                                        ] :: [    4/80132]
[ 60s, 70s) :: [                                        ] :: [    4/80132]
[ 50s, 60s) :: [                                        ] :: [    5/80132]
[ 40s, 50s) :: [                                        ] :: [   10/80132]
[ 30s, 40s) :: [                                        ] :: [   24/80132]
[ 20s, 30s) :: [                                        ] :: [   48/80132]
[ 10s, 20s) :: [                                        ] :: [  271/80132]
[  0s, 10s) :: [*************************************** ] :: [79759/80132]
--------------------------------------------------------------------------
Testing Time: 583.25s
  Skipped          :    52
  Unsupported      :   975
  Passed           : 87224
  Expectedly Failed:   191

• With the MR, it also pass, but only taking some more times

Slowest Tests:
--------------------------------------------------------------------------
155.65s: Clang :: Driver/fsanitize.c
146.70s: Clang :: Driver/arm-cortex-cpus-2.c
130.69s: Clang :: Driver/arm-cortex-cpus-1.c
104.99s: Clang :: Driver/linux-ld.c
93.88s: Clang :: OpenMP/target_update_codegen.cpp
93.66s: Clang :: OpenMP/target_defaultmap_codegen_01.cpp
84.77s: Clang :: Driver/clang_f_opts.c
81.02s: Clang :: Driver/cl-options.c
67.31s: Clang :: Driver/debug-options.c
65.48s: Clang :: Preprocessor/arm-target-features.c
64.31s: Clang :: Driver/riscv-arch.c
61.93s: Clang :: Driver/mips-fsf.cpp
61.89s: Clang :: Preprocessor/aarch64-target-features.c
59.26s: Clang :: Preprocessor/predefined-arch-macros.c
58.48s: lit :: shtest-define.py
54.39s: lit :: shtest-shell.py
50.12s: Clang :: OpenMP/target_data_codegen.cpp
48.99s: LLVM :: CodeGen/ARM/build-attributes.ll
46.24s: Clang :: Driver/x86-target-features.c
46.18s: Clang :: Driver/cl-outputs.c
Tests Times:
--------------------------------------------------------------------------
[   Range   ] :: [               Percentage               ] :: [   Count   ]
--------------------------------------------------------------------------
[150s,160s) :: [                                        ] :: [    2/80056]
[140s,150s) :: [                                        ] :: [    0/80056]
[130s,140s) :: [                                        ] :: [    1/80056]
[120s,130s) :: [                                        ] :: [    0/80056]
[110s,120s) :: [                                        ] :: [    0/80056]
[100s,110s) :: [                                        ] :: [    1/80056]
[ 90s,100s) :: [                                        ] :: [    2/80056]
[ 80s, 90s) :: [                                        ] :: [    2/80056]
[ 70s, 80s) :: [                                        ] :: [    0/80056]
[ 60s, 70s) :: [                                        ] :: [    7/80056]
[ 50s, 60s) :: [                                        ] :: [    3/80056]
[ 40s, 50s) :: [                                        ] :: [   14/80056]
[ 30s, 40s) :: [                                        ] :: [   18/80056]
[ 20s, 30s) :: [                                        ] :: [   66/80056]
[ 10s, 20s) :: [                                        ] :: [  248/80056]
[  0s, 10s) :: [*************************************** ] :: [79692/80056]
--------------------------------------------------------------------------
Testing Time: 626.41s
  Skipped          :    52
  Unsupported      :   975
  Passed           : 87225
  Expectedly Failed:   191

[Allen]

[Allen]

hi @vitalybuka, Excuse me,again.

May the issue relate to the timeout or I miss somethings to reprocduce the break ?

[Allen]

hi @vitalybuka, as the issue can't be repoduced locally, do you object to retry again ?

[vitalybuka]

In D141188#4139179, @Allen wrote:

hi @vitalybuka, as the issue can't be repoduced locally, do you object to retry again ?

I can't reproduce it as well.

[bgraur]

@Allen test unittests/ProfileData/ProfileDataTests fails when built with optimisations level -O1 with clang including this patch.

When building the test with -O3 (Release) or -O1 -g it passes.

Not sure if this patch triggers some other bug in clang or the test has UB or the patch itself is to blame.
Please revert this patch and investigate the issue without any pressure.

Repro steps:

1. Build and install clang somewhere.
2. Build the test with ninja ProfileDataTests -v. The compilation command and linker command will be displayed.
3. Edit the commands to specify -O1 and no -g. It is enough to only build the test in unittests/ProfileData/CoverageMappingTest.cpp as that one shows the failure.

The command will look something like this:

${INSTALL_PATH}/bin/clang++ \
  -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE -D_LIBCPP_ENABLE_ASSERTIONS \
  -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS \
  -I${PROJECT}/llvm-project/build/unittests/ProfileData \
  -I${PROJECT}/llvm-project/llvm/unittests/ProfileData \
  -I${PROJECT}/llvm-project/build/include \
  -I${PROJECT}/llvm-project/llvm/include \
  -I${PROJECT}/llvm-project/third-party/unittest/googletest/include \
  -I${PROJECT}/llvm-project/third-party/unittest/googlemock/include \
  -O1 -fPIC -fno-semantic-interposition -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 -Wmisleading-indentation -Wctad-maybe-unsupported \
  -fdiagnostics-color -Wno-variadic-macros -Wno-gnu-zero-variadic-macro-arguments -fno-exceptions -fno-rtti \
  -Wno-suggest-override -std=c++17 \
  -MD -MT unittests/ProfileData/CMakeFiles/ProfileDataTests.dir/CoverageMappingTest.cpp.o \
  -MF unittests/ProfileData/CMakeFiles/ProfileDataTests.dir/CoverageMappingTest.cpp.o.d \
  -o unittests/ProfileData/CMakeFiles/ProfileDataTests.dir/CoverageMappingTest.cpp.o \
  -c ${PROJECT}/llvm-project/llvm/unittests/ProfileData/CoverageMappingTest.cpp

${INSTALL_PATH}/bin/clang++ -O1 -fPIC -fno-semantic-interposition \
  -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 -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color \
  unittests/ProfileData/CMakeFiles/ProfileDataTests.dir/CoverageMappingTest.cpp.o \
  -o unittests/ProfileData/ProfileDataTests  \
  lib/libLLVMCore.a  lib/libLLVMCoverage.a  lib/libLLVMProfileData.a  lib/libLLVMSupport.a  lib/libLLVMObject.a  \
  lib/libLLVMSupport.a  lib/libllvm_gtest_main.a  lib/libllvm_gtest.a  lib/libLLVMTestingSupport.a  \
  lib/libLLVMSymbolize.a  lib/libLLVMDebugInfoPDB.a  lib/libLLVMDebugInfoMSF.a  lib/libLLVMDebugInfoDWARF.a  \
  lib/libLLVMObject.a  lib/libLLVMIRReader.a  lib/libLLVMBitReader.a  lib/libLLVMAsmParser.a  lib/libLLVMCore.a  \
  lib/libLLVMRemarks.a  lib/libLLVMBitstreamReader.a  lib/libLLVMMCParser.a  lib/libLLVMMC.a  lib/libLLVMDebugInfoCodeView.a  \
  lib/libLLVMTextAPI.a  lib/libLLVMBinaryFormat.a  lib/libLLVMTargetParser.a  lib/libllvm_gtest.a  lib/libLLVMSupport.a  \
  lib/libLLVMDemangle.a  -lrt  -ldl  -lm  /usr/lib/x86_64-linux-gnu/libz.so  /usr/lib/x86_64-linux-gnu/libtinfo.so  -lpthread 

$ unittests/ProfileData/ProfileDataTests
..........................
[==========] 118 tests from 2 test suites ran. (4 ms total)
[  PASSED  ] 114 tests.
[  FAILED  ] 4 tests, listed below:
[  FAILED  ] ParameterizedCovMapTest/CoverageMappingTest.test_line_coverage_iterator/0, where GetParam() = (false, false)
[  FAILED  ] ParameterizedCovMapTest/CoverageMappingTest.test_line_coverage_iterator/1, where GetParam() = (false, true)
[  FAILED  ] ParameterizedCovMapTest/CoverageMappingTest.test_line_coverage_iterator/2, where GetParam() = (true, false)
[  FAILED  ] ParameterizedCovMapTest/CoverageMappingTest.test_line_coverage_iterator/3, where GetParam() = (true, true)

[Allen]

In D141188#4161731, @bgraur wrote:

@Allen test unittests/ProfileData/ProfileDataTests fails when built with optimisations level -O1 with clang including this patch.

When building the test with -O3 (Release) or -O1 -g it passes.

Not sure if this patch triggers some other bug in clang or the test has UB or the patch itself is to blame.
Please revert this patch and investigate the issue without any pressure.

Thanks for your report, I'll take a look at that and now revert with commit af2969fd46b7

[ayzhao]

FYI this patch also caused some of Chrome's unit tests to fail - see https://crbug.com/1420824

[glandium]

FYI this patch also caused some of Firefox's unit tests to fail, e.g. https://treeherder.mozilla.org/logviewer?job_id=407511605&repo=toolchains&lineNumber=32470

[Allen]

For the runtime fails of unittests/ProfileData/ProfileDataTests, there is 2 points important

• a) build without -g fall as it transform code (From function _ZN12_GLOBAL__N_152CoverageMappingTest_test_line_coverage_iterator_Test8TestBodyEv)

_ZNK4llvm14iterator_rangeINS_8coverage20LineCoverageIteratorEE3endEv.exit: ; preds = %_ZNK4llvm14iterator_rangeINS_8coverage20LineCoverageIteratorEE5beginEv.exit, %if.then.i.i.i294
  %Stats.i.i295 = getelementptr inbounds %"class.llvm::coverage::LineCoverageIterator", ptr %__end1, i64 0, i32 6
  %Stats4.i.i296 = getelementptr inbounds %"class.llvm::iterator_range", ptr %ref.tmp32, i64 0, i32 1, i32 6
  call void @llvm.memcpy.p0.p0.i64(ptr noundef nonnull align 8 dereferenceable(40) %Stats.i.i295, ptr noundef nonnull align 8 dereferenceable(40) %Stats4.i.i296, i64 40, i1 false), !tbaa.struct !339
  %Next.i.i299 = getelementptr inbounds %"class.llvm::coverage::LineCoverageIterator", ptr %__begin1, i64 0, i32 2
  %Next3.i.i = getelementptr inbounds %"class.llvm::coverage::LineCoverageIterator", ptr %__end1, i64 0, i32 2
  %Ended.i.i301 = getelementptr inbounds %"class.llvm::coverage::LineCoverageIterator", ptr %__begin1, i64 0, i32 3
  %Ended4.i.i = getelementptr inbounds %"class.llvm::coverage::LineCoverageIterator", ptr %__end1, i64 0, i32 3
  %Line.i = getelementptr inbounds %"class.llvm::coverage::LineCoverageIterator", ptr %__begin1, i64 0, i32 6, i32 3
  %message_.i320 = getelementptr inbounds %"class.testing::AssertionResult", ptr %gtest_ar35, i64 0, i32 1
  %message_.i386 = getelementptr inbounds %"class.testing::AssertionResult", ptr %gtest_ar55, i64 0, i32 1
  br label %for.cond

for.cond:                                         ; preds = %_ZN7testing15AssertionResultD2Ev.exit392, %_ZNK4llvm14iterator_rangeINS_8coverage20LineCoverageIteratorEE3endEv.exit
  %340 = load ptr, ptr %__begin1, align 8, !tbaa !340
  %341 = load ptr, ptr %__end1, align 8, !tbaa !340
  %cmp.i.i298 = icmp eq ptr %340, %341
  br i1 %cmp.i.i298, label %land.lhs.true.i.i, label %_ZNK4llvm20iterator_facade_baseINS_8coverage20LineCoverageIteratorESt20forward_iterator_tagKNS1_17LineCoverageStatsElPS5_RS5_EneERKS2_.exit

land.lhs.true.i.i:                                ; preds = %for.cond
  %342 = load ptr, ptr %Next.i.i299, align 8, !tbaa !48
  %343 = load ptr, ptr %Next3.i.i, align 8, !tbaa !48
  %cmp.i.i.i300 = icmp eq ptr %342, %343
  br i1 %cmp.i.i.i300, label %land.rhs.i.i, label %_ZNK4llvm20iterator_facade_baseINS_8coverage20LineCoverageIteratorESt20forward_iterator_tagKNS1_17LineCoverageStatsElPS5_RS5_EneERKS2_.exit

land.rhs.i.i:                                     ; preds = %land.lhs.true.i.i
  %344 = load i8, ptr %Ended.i.i301, align 8, !tbaa !333, !range !18, !noundef !19
  %345 = load i8, ptr %Ended4.i.i, align 8, !tbaa !333, !range !18, !noundef !19
  %cmp7.i.i = icmp ne i8 %344, %345
  br label %_ZNK4llvm20iterator_facade_baseINS_8coverage20LineCoverageIteratorESt20forward_iterator_tagKNS1_17LineCoverageStatsElPS5_RS5_EneERKS2_.exit

_ZNK4llvm20iterator_facade_baseINS_8coverage20LineCoverageIteratorESt20forward_iterator_tagKNS1_17LineCoverageStatsElPS5_RS5_EneERKS2_.exit: ; preds = %for.cond, %land.lhs.true.i.i, %land.rhs.i.i
  %lnot.i = phi i1 [ true, %land.lhs.true.i.i ], [ true, %for.cond ], [ %cmp7.i.i, %land.rhs.i.i ]
  br i1 %lnot.i, label %for.body, label %for.end.critedge

into a %memcmp, and the detail code shows as following (The first %340 = icmp ne ptr %338, %339 should be keep using condition eq )

_ZNK4llvm14iterator_rangeINS_8coverage20LineCoverageIteratorEE3endEv.exit: ; preds = %_ZNK4llvm14iterator_rangeINS_8coverage20LineCoverageIteratorEE5beginEv.exit, %if.then.i.i.i294
  %Stats.i.i295 = getelementptr inbounds %"class.llvm::coverage::LineCoverageIterator", ptr %__end1, i64 0, i32 6
  %Stats4.i.i296 = getelementptr inbounds %"class.llvm::iterator_range", ptr %ref.tmp32, i64 0, i32 1, i32 6
  call void @llvm.memcpy.p0.p0.i64(ptr noundef nonnull align 8 dereferenceable(40) %Stats.i.i295, ptr noundef nonnull align 8 dereferenceable(40) %Stats4.i.i296, i64 40, i1 false), !tbaa.struct !339
  %Next.i.i299 = getelementptr inbounds %"class.llvm::coverage::LineCoverageIterator", ptr %__begin1, i64 0, i32 2
  %Next3.i.i = getelementptr inbounds %"class.llvm::coverage::LineCoverageIterator", ptr %__end1, i64 0, i32 2
  %Ended.i.i301 = getelementptr inbounds %"class.llvm::coverage::LineCoverageIterator", ptr %__begin1, i64 0, i32 3
  %Ended4.i.i = getelementptr inbounds %"class.llvm::coverage::LineCoverageIterator", ptr %__end1, i64 0, i32 3
  %Line.i = getelementptr inbounds %"class.llvm::coverage::LineCoverageIterator", ptr %__begin1, i64 0, i32 6, i32 3
  %message_.i320 = getelementptr inbounds %"class.testing::AssertionResult", ptr %gtest_ar35, i64 0, i32 1
  %message_.i386 = getelementptr inbounds %"class.testing::AssertionResult", ptr %gtest_ar55, i64 0, i32 1
  br label %for.cond582

for.cond582:                                      ; preds = %_ZNK4llvm14iterator_rangeINS_8coverage20LineCoverageIteratorEE3endEv.exit, %_ZN7testing15AssertionResultD2Ev.exit392
  %338 = load ptr, ptr %__begin1, align 8
  %339 = load ptr, ptr %__end1, align 8
  %340 = icmp ne ptr %338, %339
  br i1 %340, label %"land.lhs.true.i.i+land.rhs.i.i", label %_ZNK4llvm20iterator_facade_baseINS_8coverage20LineCoverageIteratorESt20forward_iterator_tagKNS1_17LineCoverageStatsElPS5_RS5_EneERKS2_.exit

"land.lhs.true.i.i+land.rhs.i.i":                 ; preds = %for.cond582
  %341 = getelementptr inbounds %"class.llvm::coverage::LineCoverageIterator", ptr %__begin1, i64 0, i32 2
  %342 = getelementptr inbounds %"class.llvm::coverage::LineCoverageIterator", ptr %__end1, i64 0, i32 2
  %memcmp = call i32 @memcmp(ptr %341, ptr %342, i64 9)
  %343 = icmp ne i32 %memcmp, 0
  br label %_ZNK4llvm20iterator_facade_baseINS_8coverage20LineCoverageIteratorESt20forward_iterator_tagKNS1_17LineCoverageStatsElPS5_RS5_EneERKS2_.exit

_ZNK4llvm20iterator_facade_baseINS_8coverage20LineCoverageIteratorESt20forward_iterator_tagKNS1_17LineCoverageStatsElPS5_RS5_EneERKS2_.exit: ; preds = %for.cond582, %"land.lhs.true.i.i+land.rhs.i.i"
  %lnot.i = phi i1 [ %343, %"land.lhs.true.i.i+land.rhs.i.i" ], [ false, %for.cond582 ]
  br i1 %lnot.i, label %for.body, label %for.end.critedge

2. b) build with -g works fine is because Comparison->doesOtherWork() in function BCECmpChain::BCECmpChain return false （the transform doesn't active） as some call void @llvm.dbg.value, which is not exist when build without -g.

[Allen]

Fix the runtime issue related to test unittests/ProfileData/ProfileDataTests.

The issue also can be reproduced on aarch64 cpu, the failure is caused by
the new spilted Icmp chain from not-equal comparisons chain is end with
equal condition, I am not aware of this before.

[Allen]

In D141188#4166296, @glandium wrote:

FYI this patch also caused some of Firefox's unit tests to fail, e.g. https://treeherder.mozilla.org/logviewer?job_id=407511605&repo=toolchains&lineNumber=32470

hi @glandium, As I don't know how to check the above tests, would you help me to check that base on my new update changes ?

[ayzhao]

I tested the updated version of this patch to see if it fixes the Chrome test failures. Unfortunately, Clang now crashes when compiling an unrelated file in Chrome:

[5373/37986] CXX obj/third_party/angle/translator/ConvertUnsupportedConstructorsToFuncti[11272/37597] CXX obj/skia/skia/GrQuadUtils.o                                           FAILED: obj/skia/skia/GrQuadUtils.o
/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang++ -MMD -MF obj/skia/skia/GrQuadUtils.o.d -DDCHECK_ALWAYS_ON=1 -DUSE_UDEV -DUSE_AURA=1 -DUSE_GLIB=1 -DUSE_OZONE=1 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_GNU_SOURCE -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -D_LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS -DCR_LIBCXX_REVISION=3cf0fe4445843426d7c51eca9daad9ae0c2ff77d -D_LIBCPP_ENABLE_ASSERTIONS=1 -DCR_SYSROOT_KEY=20221105T211506Z-0 -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DSK_CODEC_DECODES_PNG -DSK_CODEC_DECODES_WEBP -DSK_ENCODE_PNG -DSK_ENCODE_WEBP -DSK_ENABLE_SKSL -DSK_UNTIL_CRBUG_1187654_IS_FIXED -DSK_USER_CONFIG_HEADER=\"../../skia/config/SkUserConfig.h\" -DSK_WIN_FONTMGR_NO_SIMULATIONS -DSK_GL -DSK_CODEC_DECODES_JPEG -DSK_ENCODE_JPEG -DSK_HAS_WUFFS_LIBRARY -DSK_VULKAN=1 -DSK_GANESH -DSK_GPU_WORKAROUNDS_HEADER=\"gpu/config/gpu_driver_bug_workaround_autogen.h\" -DVK_USE_PLATFORM_XCB_KHR -DVK_USE_PLATFORM_WAYLAND_KHR -DIS_SKIA_IMPL=1 -DSKIA_IMPLEMENTATION=1 -DSK_FREETYPE_MINIMUM_RUNTIME_VERSION=\(\(\(FREETYPE_MAJOR\)\ \*\ 0x01000000\)\ \|\ \(\(FREETYPE_MINOR\)\ \*\ 0x00010000\)\ \|\ \(\(FREETYPE_PATCH\)\ \*\ 0x00000100\)\) -DSK_TYPEFACE_FACTORY_FREETYPE -DSK_GAMMA_EXPONENT=1.2 -DSK_GAMMA_CONTRAST=0.2 -DSK_DEFAULT_FONT_CACHE_LIMIT=20971520 -DGLIB_VERSION_MAX_ALLOWED=GLIB_VERSION_2_56 -DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_56 -DWEBP_EXTERN=extern -DFT_CONFIG_MODULES_H=\"freetype-custom/freetype/config/ftmodule.h\" -DFT_CONFIG_OPTIONS_H=\"freetype-custom/freetype/config/ftoption.h\" -DPDFIUM_REQUIRED_MODULES -DCHROMIUM_RESTRICT_VISIBILITY -DUSE_LIBJPEG_TURBO=1 -DMANGLE_JPEG_NAMES -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 -DUSE_CHROMIUM_ICU=1 -DU_ENABLE_TRACING=1 -DU_ENABLE_RESOURCE_TRACING=0 -DU_STATIC_IMPLEMENTATION -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -I../.. -Igen -I../../buildtools/third_party/libc++ -I../../third_party/skia -I../../third_party/wuffs/src/release/c -I../../third_party/vulkan/include -I../../third_party/vulkan-deps/vulkan-headers/src/include -I../../third_party/wayland/src/src -I../../third_party/wayland/include/src -I../../third_party/perfetto/include -Igen/third_party/perfetto/build_config -Igen/third_party/perfetto -I../../third_party/libwebp/src/src -I../../third_party/abseil-cpp -I../../third_party/boringssl/src/include -I../../third_party/protobuf/src -Igen/protoc_out -I../../third_party/libpng -I../../third_party/zlib -I../../third_party/freetype/include -I../../third_party/freetype/include/freetype-custom -I../../third_party/freetype/src/include -I../../third_party/harfbuzz-ng/src/src -I../../third_party/libjpeg_turbo -I../../third_party/fontconfig/src -I../../third_party/icu/source/common -I../../third_party/icu/source/i18n -fno-delete-null-pointer-checks -fno-ident -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector -funwind-tables -fPIC -pthread -fcolor-diagnostics -fmerge-all-constants -fcrash-diagnostics-dir=../../tools/clang/crashreports -mllvm -instcombine-lower-dbg-declare=0 -ffp-contract=off -fcomplete-member-pointers -m64 -msse3 -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -ffile-compilation-dir=. -no-canonical-prefixes -ftrivial-auto-var-init=pattern -fno-omit-frame-pointer -g0 -fvisibility=hidden -Wno-redundant-parens -Werror -Wall -Wno-unused-variable -Wno-c++11-narrowing -Wno-unused-but-set-variable -Wno-misleading-indentation -Wno-missing-field-initializers -Wno-unused-parameter -Wno-psabi -Wloop-analysis -Wno-unneeded-internal-declaration -Wenum-compare-conditional -Wno-ignored-pragma-optimize -Wno-deprecated-builtins -Wno-bitfield-constant-conversion -Wno-deprecated-this-capture -O2 -fdata-sections -ffunction-sections -fno-unique-section-names -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/glib-2.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/lib/x86_64-linux-gnu/glib-2.0/include -DPROTOBUF_ALLOW_DEPRECATED=1 -std=c++20 -Wno-trigraphs -gsimple-template-names -fno-exceptions -fno-rtti -nostdinc++ -isystem../../buildtools/third_party/libc++/trunk/include -isystem../../buildtools/third_party/libc++abi/trunk/include --sysroot=../../build/linux/debian_bullseye_amd64-sysroot -fvisibility-inlines-hidden -c ../../third_party/skia/src/gpu/ganesh/geometry/GrQuadUtils.cpp -o obj/skia/skia/GrQuadUtils.o
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.      Program arguments: /usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang++ -MMD -MF obj/skia/skia/GrQuadUtils.o.d -DDCHECK_ALWAYS_ON=1 -DUSE_UDEV -DUSE_AURA=1 -DUSE_GLIB=1 -DUSE_OZONE=1 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_GNU_SOURCE -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -D_LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS -DCR_LIBCXX_REVISION=3cf0fe4445843426d7c51eca9daad9ae0c2ff77d -D_LIBCPP_ENABLE_ASSERTIONS=1 -DCR_SYSROOT_KEY=20221105T211506Z-0 -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DSK_CODEC_DECODES_PNG -DSK_CODEC_DECODES_WEBP -DSK_ENCODE_PNG -DSK_ENCODE_WEBP -DSK_ENABLE_SKSL -DSK_UNTIL_CRBUG_1187654_IS_FIXED -DSK_USER_CONFIG_HEADER=\"../../skia/config/SkUserConfig.h\" -DSK_WIN_FONTMGR_NO_SIMULATIONS -DSK_GL -DSK_CODEC_DECODES_JPEG -DSK_ENCODE_JPEG -DSK_HAS_WUFFS_LIBRARY -DSK_VULKAN=1 -DSK_GANESH -DSK_GPU_WORKAROUNDS_HEADER=\"gpu/config/gpu_driver_bug_workaround_autogen.h\" -DVK_USE_PLATFORM_XCB_KHR -DVK_USE_PLATFORM_WAYLAND_KHR -DIS_SKIA_IMPL=1 -DSKIA_IMPLEMENTATION=1 "-DSK_FREETYPE_MINIMUM_RUNTIME_VERSION=(((FREETYPE_MAJOR) * 0x01000000) | ((FREETYPE_MINOR) * 0x00010000) | ((FREETYPE_PATCH) * 0x00000100))" -DSK_TYPEFACE_FACTORY_FREETYPE -DSK_GAMMA_EXPONENT=1.2 -DSK_GAMMA_CONTRAST=0.2 -DSK_DEFAULT_FONT_CACHE_LIMIT=20971520 -DGLIB_VERSION_MAX_ALLOWED=GLIB_VERSION_2_56 -DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_56 -DWEBP_EXTERN=extern -DFT_CONFIG_MODULES_H=\"freetype-custom/freetype/config/ftmodule.h\" -DFT_CONFIG_OPTIONS_H=\"freetype-custom/freetype/config/ftoption.h\" -DPDFIUM_REQUIRED_MODULES -DCHROMIUM_RESTRICT_VISIBILITY -DUSE_LIBJPEG_TURBO=1 -DMANGLE_JPEG_NAMES -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 -DUSE_CHROMIUM_ICU=1 -DU_ENABLE_TRACING=1 -DU_ENABLE_RESOURCE_TRACING=0 -DU_STATIC_IMPLEMENTATION -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -I../.. -Igen -I../../buildtools/third_party/libc++ -I../../third_party/skia -I../../third_party/wuffs/src/release/c -I../../third_party/vulkan/include -I../../third_party/vulkan-deps/vulkan-headers/src/include -I../../third_party/wayland/src/src -I../../third_party/wayland/include/src -I../../third_party/perfetto/include -Igen/third_party/perfetto/build_config -Igen/third_party/perfetto -I../../third_party/libwebp/src/src -I../../third_party/abseil-cpp -I../../third_party/boringssl/src/include -I../../third_party/protobuf/src -Igen/protoc_out -I../../third_party/libpng -I../../third_party/zlib -I../../third_party/freetype/include -I../../third_party/freetype/include/freetype-custom -I../../third_party/freetype/src/include -I../../third_party/harfbuzz-ng/src/src -I../../third_party/libjpeg_turbo -I../../third_party/fontconfig/src -I../../third_party/icu/source/common -I../../third_party/icu/source/i18n -fno-delete-null-pointer-checks -fno-ident -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector -funwind-tables -fPIC -pthread -fcolor-diagnostics -fmerge-all-constants -fcrash-diagnostics-dir=../../tools/clang/crashreports -mllvm -instcombine-lower-dbg-declare=0 -ffp-contract=off -fcomplete-member-pointers -m64 -msse3 -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -ffile-compilation-dir=. -no-canonical-prefixes -ftrivial-auto-var-init=pattern -fno-omit-frame-pointer -g0 -fvisibility=hidden -Wno-redundant-parens -Werror -Wall -Wno-unused-variable -Wno-c++11-narrowing -Wno-unused-but-set-variable -Wno-misleading-indentation -Wno-missing-field-initializers -Wno-unused-parameter -Wno-psabi -Wloop-analysis -Wno-unneeded-internal-declaration -Wenum-compare-conditional -Wno-ignored-pragma-optimize -Wno-deprecated-builtins -Wno-bitfield-constant-conversion -Wno-deprecated-this-capture -O2 -fdata-sections -ffunction-sections -fno-unique-section-names -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/glib-2.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/lib/x86_64-linux-gnu/glib-2.0/include -DPROTOBUF_ALLOW_DEPRECATED=1 -std=c++20 -Wno-trigraphs -gsimple-template-names -fno-exceptions -fno-rtti -nostdinc++ -isystem../../buildtools/third_party/libc++/trunk/include -isystem../../buildtools/third_party/libc++abi/trunk/include --sysroot=../../build/linux/debian_bullseye_amd64-sysroot -fvisibility-inlines-hidden -c ../../third_party/skia/src/gpu/ganesh/geometry/GrQuadUtils.cpp -o obj/skia/skia/GrQuadUtils.o
1.      <eof> parser at end of file
2.      Optimizer
 #0 0x000055b398f7c297 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x3ef5297)
 #1 0x000055b398f7a14e llvm::sys::RunSignalHandlers() (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x3ef314e)
 #2 0x000055b398ef7d48 CrashRecoverySignalHandler(int) CrashRecoveryContext.cpp:0:0
 #3 0x00007fc93b45af90 (/lib/x86_64-linux-gnu/libc.so.6+0x3bf90)
 #4 0x000055b397d6606e llvm::DataLayout::getTypeSizeInBits(llvm::Type*) const X86ISelLowering.cpp:0:0
 #5 0x000055b398e619d5 llvm::SROAPass::rewritePartition(llvm::AllocaInst&, llvm::sroa::AllocaSlices&, llvm::sroa::Partition&) (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x3dda9d5)
 #6 0x000055b398e65788 llvm::SROAPass::splitAlloca(llvm::AllocaInst&, llvm::sroa::AllocaSlices&) (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x3dde788)
 #7 0x000055b398e68e21 llvm::SROAPass::runOnAlloca(llvm::AllocaInst&) (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x3de1e21)
 #8 0x000055b398e6c19e llvm::SROAPass::runImpl(llvm::Function&, llvm::DomTreeUpdater&, llvm::AssumptionCache&) (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x3de519e)
 #9 0x000055b398e6d361 llvm::SROAPass::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x3de6361)
#10 0x000055b39a38883d llvm::detail::PassModel<llvm::Function, llvm::SROAPass, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) PassBuilder.cpp:0:0
#11 0x000055b3989e725e llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x396025e)
#12 0x000055b39984522d llvm::detail::PassModel<llvm::Function, llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) BackendUtil.cpp:0:0
#13 0x000055b398a7a0c8 llvm::CGSCCToFunctionPassAdaptor::run(llvm::LazyCallGraph::SCC&, llvm::AnalysisManager<llvm::LazyCallGraph::SCC, llvm::LazyCallGraph&>&, llvm::LazyCallGraph&, llvm::CGSCCUpdateResult&) (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x39f30c8)
#14 0x000055b39a38a29d llvm::detail::PassModel<llvm::LazyCallGraph::SCC, llvm::CGSCCToFunctionPassAdaptor, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::LazyCallGraph::SCC, llvm::LazyCallGraph&>, llvm::LazyCallGraph&, llvm::CGSCCUpdateResult&>::run(llvm::LazyCallGraph::SCC&, llvm::AnalysisManager<llvm::LazyCallGraph::SCC, llvm::LazyCallGraph&>&, llvm::LazyCallGraph&, llvm::CGSCCUpdateResult&) PassBuilder.cpp:0:0
#15 0x000055b398a74bc4 llvm::PassManager<llvm::LazyCallGraph::SCC, llvm::AnalysisManager<llvm::LazyCallGraph::SCC, llvm::LazyCallGraph&>, llvm::LazyCallGraph&, llvm::CGSCCUpdateResult&>::run(llvm::LazyCallGraph::SCC&, llvm::AnalysisManager<llvm::LazyCallGraph::SCC, llvm::LazyCallGraph&>&, llvm::LazyCallGraph&, llvm::CGSCCUpdateResult&) (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x39edbc4)
#16 0x000055b398a97c0d llvm::detail::PassModel<llvm::LazyCallGraph::SCC, llvm::PassManager<llvm::LazyCallGraph::SCC, llvm::AnalysisManager<llvm::LazyCallGraph::SCC, llvm::LazyCallGraph&>, llvm::LazyCallGraph&, llvm::CGSCCUpdateResult&>, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::LazyCallGraph::SCC, llvm::LazyCallGraph&>, llvm::LazyCallGraph&, llvm::CGSCCUpdateResult&>::run(llvm::LazyCallGraph::SCC&, llvm::AnalysisManager<llvm::LazyCallGraph::SCC, llvm::LazyCallGraph&>&, llvm::LazyCallGraph&, llvm::CGSCCUpdateResult&) Inliner.cpp:0:0
#17 0x000055b398a782a9 llvm::DevirtSCCRepeatedPass::run(llvm::LazyCallGraph::SCC&, llvm::AnalysisManager<llvm::LazyCallGraph::SCC, llvm::LazyCallGraph&>&, llvm::LazyCallGraph&, llvm::CGSCCUpdateResult&) (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x39f12a9)
#18 0x000055b398a9844d llvm::detail::PassModel<llvm::LazyCallGraph::SCC, llvm::DevirtSCCRepeatedPass, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::LazyCallGraph::SCC, llvm::LazyCallGraph&>, llvm::LazyCallGraph&, llvm::CGSCCUpdateResult&>::run(llvm::LazyCallGraph::SCC&, llvm::AnalysisManager<llvm::LazyCallGraph::SCC, llvm::LazyCallGraph&>&, llvm::LazyCallGraph&, llvm::CGSCCUpdateResult&) Inliner.cpp:0:0
#19 0x000055b398a76dca llvm::ModuleToPostOrderCGSCCPassAdaptor::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x39efdca)
#20 0x000055b398a981ed llvm::detail::PassModel<llvm::Module, llvm::ModuleToPostOrderCGSCCPassAdaptor, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) Inliner.cpp:0:0
#21 0x000055b3989e5eae llvm::PassManager<llvm::Module, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x395eeae)
#22 0x000055b398a93afc llvm::ModuleInlinerWrapperPass::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x3a0cafc)
#23 0x000055b39a37a62d llvm::detail::PassModel<llvm::Module, llvm::ModuleInlinerWrapperPass, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) PassBuilder.cpp:0:0
#24 0x000055b3989e5eae llvm::PassManager<llvm::Module, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x395eeae)
#25 0x000055b39983d5d9 (anonymous namespace)::EmitAssemblyHelper::RunOptimizationPipeline(clang::BackendAction, std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream>>&, std::unique_ptr<llvm::ToolOutputFile, std::default_delete<llvm::ToolOutputFile>>&) BackendUtil.cpp:0:0
#26 0x000055b399834371 clang::EmitBackendOutput(clang::DiagnosticsEngine&, clang::HeaderSearchOptions const&, clang::CodeGenOptions const&, clang::TargetOptions const&, clang::LangOptions const&, llvm::StringRef, llvm::Module*, clang::BackendAction, llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>, std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream>>) (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x47ad371)
#27 0x000055b399ccb174 clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&) CodeGenAction.cpp:0:0
#28 0x000055b39b255f24 clang::ParseAST(clang::Sema&, bool, bool) (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x61cef24)
#29 0x000055b399be2800 clang::FrontendAction::Execute() (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x4b5b800)
#30 0x000055b399b4e09f clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x4ac709f)
#31 0x000055b399cc3f8f clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x4c3cf8f)
#32 0x000055b397cef733 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x2c68733)
#33 0x000055b397ceb4e1 ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&, llvm::ToolContext const&) driver.cpp:0:0
#34 0x000055b3999c6e29 void llvm::function_ref<void ()>::callback_fn<clang::driver::CC1Command::Execute(llvm::ArrayRef<std::optional<llvm::StringRef>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, bool*) const::$_1>(long) Job.cpp:0:0
#35 0x000055b398ef7a8b llvm::CrashRecoveryContext::RunSafely(llvm::function_ref<void ()>) (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x3e70a8b)
#36 0x000055b3999c6280 clang::driver::CC1Command::Execute(llvm::ArrayRef<std::optional<llvm::StringRef>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, bool*) const (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x493f280)
#37 0x000055b399984dde clang::driver::Compilation::ExecuteCommand(clang::driver::Command const&, clang::driver::Command const*&, bool) const (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x48fddde)
#38 0x000055b399985097 clang::driver::Compilation::ExecuteJobs(clang::driver::JobList const&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command const*>>&, bool) const (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x48fe097)
#39 0x000055b3999a4919 clang::driver::Driver::ExecuteCompilation(clang::driver::Compilation&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command const*>>&) (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x491d919)
#40 0x000055b397ceaa58 clang_main(int, char**, llvm::ToolContext const&) (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x2c63a58)
#41 0x000055b397cfbed1 main (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x2c74ed1)
#42 0x00007fc93b44618a __libc_start_call_main ./csu/../sysdeps/nptl/libc_start_call_main.h:74:3
#43 0x00007fc93b446245 call_init ./csu/../csu/libc-start.c:128:20
#44 0x00007fc93b446245 __libc_start_main ./csu/../csu/libc-start.c:368:5
#45 0x000055b397ce7521 _start (/usr/local/google/home/ayzhao/src/llvm-project/build/bin/clang+++0x2c60521)
clang++: error: clang frontend command failed with exit code 139 (use -v to see invocation)
clang version 17.0.0 ([e-mail]:llvm/llvm-project.git 55d5410442f446e4edf1c4ba850d24631b7b6e80)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/local/google/home/ayzhao/src/llvm-project/build/bin
clang++: note: diagnostic msg:
********************

PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
Preprocessed source(s) and associated run script(s) are located at:
clang++: note: diagnostic msg: ../../tools/clang/crashreports/GrQuadUtils-ab2f11.cpp
clang++: note: diagnostic msg: ../../tools/clang/crashreports/GrQuadUtils-ab2f11.sh
clang++: note: diagnostic msg:

********************
[11401/37597] CXX obj/skia/skia/GrGLGpu.o
ninja: build stopped: subcommand failed.

File in question: https://crsrc.org/c/third_party/skia/src/gpu/ganesh/geometry/GrQuadUtils.cpp

[glandium]

In D141188#4174664, @Allen wrote:

In D141188#4166296, @glandium wrote:

FYI this patch also caused some of Firefox's unit tests to fail, e.g. https://treeherder.mozilla.org/logviewer?job_id=407511605&repo=toolchains&lineNumber=32470

hi @glandium, As I don't know how to check the above tests, would you help me to check that base on my new update changes ?

Building Firefox with this new patch applied yields a clang crash like @ayzhao's.

[Allen]

Thanks @glandium and @ayzhao, I'm trying to reproduce the above issue with chromium's code

[glandium]

The crash is actually happening without the patch too. It happens both in Firefox and Chrome because both are using Skia, and the source that triggers the crash is in Skia.

[glandium]

The crash comes from D143225.

[Allen]

In D141188#4180343, @glandium wrote:

The crash comes from D143225.

Oh, thanks @glandium very much for your information.
In that case, I'll land this patch in the near future, as the previous runtime issue has been resolved.

[glandium]

In D141188#4180432, @Allen wrote:

In D141188#4180343, @glandium wrote:

The crash comes from D143225.

Oh, thanks @glandium very much for your information.
In that case, I'll land this patch in the near future, as the previous runtime issue has been resolved.

I'm testing it against trunk minus the patch that causes the crash. In a couple hours or less, I'll be able to tell you whether the latest version or the patch works properly for Firefox.

[glandium]

In D141188#4180487, @glandium wrote:

I'm testing it against trunk minus the patch that causes the crash. In a couple hours or less, I'll be able to tell you whether the latest version or the patch works properly for Firefox.

It seems good. Thanks.

[Allen]

Thank for your carefully examine.

[ayzhao]

I'm continuing to see the same unit test failure as https://crbug.com/1420824#c2 after including the latest commit

[Allen]

In D141188#4182953, @ayzhao wrote:

I'm continuing to see the same unit test failure as https://crbug.com/1420824#c2 after including the latest commit

Are the symptoms and reproduction methods consistent with those mentioned earlier? If so, I'll also try to analyze it based on the latest commit.

[aeubanks]

miscompile:

$ cat a.ll
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"                                                                                                       
target triple = "x86_64-unknown-linux-gnu"                                                                                                                                                         
                                                                                                                                                                                                   
%"struct.media::WebrtcVideoStatsDB::VideoDescKey" = type { i8, i32, i8, i32 }                                                                                                                      
                                                                                                                                                                                                   
define i1 @_ZN5medianeERKNS_18WebrtcVideoStatsDB12VideoDescKeyES3_(ptr nocapture noundef readonly align 4 dereferenceable(16) %0, ptr nocapture noundef readonly align 4 dereferenceable(16) %1) { 
  %3 = load i8, ptr %0, align 4, !range !4, !noundef !5
  %4 = load i8, ptr %1, align 4, !range !4, !noundef !5
  %5 = icmp eq i8 %3, %4
  br i1 %5, label %6, label %24

6:                                                ; preds = %2
  %7 = getelementptr inbounds %"struct.media::WebrtcVideoStatsDB::VideoDescKey", ptr %0, i64 0, i32 1
  %8 = load i32, ptr %7, align 4
  %9 = getelementptr inbounds %"struct.media::WebrtcVideoStatsDB::VideoDescKey", ptr %1, i64 0, i32 1
  %10 = load i32, ptr %9, align 4
  %11 = icmp eq i32 %8, %10
  br i1 %11, label %12, label %24

12:                                               ; preds = %6
  %13 = getelementptr inbounds %"struct.media::WebrtcVideoStatsDB::VideoDescKey", ptr %0, i64 0, i32 2
  %14 = load i8, ptr %13, align 4, !range !4, !noundef !5
  %15 = getelementptr inbounds %"struct.media::WebrtcVideoStatsDB::VideoDescKey", ptr %1, i64 0, i32 2
  %16 = load i8, ptr %15, align 4, !range !4, !noundef !5
  %17 = icmp eq i8 %14, %16
  br i1 %17, label %18, label %24

18:                                               ; preds = %12
  %19 = getelementptr inbounds %"struct.media::WebrtcVideoStatsDB::VideoDescKey", ptr %0, i64 0, i32 3
  %20 = load i32, ptr %19, align 4
  %21 = getelementptr inbounds %"struct.media::WebrtcVideoStatsDB::VideoDescKey", ptr %1, i64 0, i32 3
  %22 = load i32, ptr %21, align 4
  %23 = icmp ne i32 %20, %22
  br label %24

24:                                               ; preds = %18, %12, %6, %2
  %25 = phi i1 [ true, %12 ], [ true, %6 ], [ true, %2 ], [ %23, %18 ]
  ret i1 %25
}

!4 = !{i8 0, i8 2}
!5 = !{}
$ opt -passes=mergeicmps -S a.ll
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

%"struct.media::WebrtcVideoStatsDB::VideoDescKey" = type { i8, i32, i8, i32 }

define i1 @_ZN5medianeERKNS_18WebrtcVideoStatsDB12VideoDescKeyES3_(ptr nocapture noundef readonly align 4 dereferenceable(16) %0, ptr nocapture noundef readonly align 4 dereferenceable(16) %1) {
  %3 = load i8, ptr %0, align 1
  %4 = load i8, ptr %1, align 1
  %5 = icmp eq i8 %3, %4
  br i1 %5, label %6, label %16

6:                                                ; preds = %2
  %7 = getelementptr inbounds %"struct.media::WebrtcVideoStatsDB::VideoDescKey", ptr %0, i64 0, i32 1
  %8 = getelementptr inbounds %"struct.media::WebrtcVideoStatsDB::VideoDescKey", ptr %1, i64 0, i32 1
  %memcmp = call i32 @memcmp(ptr %7, ptr %8, i64 5)
  %9 = icmp eq i32 %memcmp, 0
  br i1 %9, label %10, label %16

10:                                               ; preds = %6
  %11 = getelementptr inbounds %"struct.media::WebrtcVideoStatsDB::VideoDescKey", ptr %0, i64 0, i32 3
  %12 = getelementptr inbounds %"struct.media::WebrtcVideoStatsDB::VideoDescKey", ptr %1, i64 0, i32 3
  %13 = load i32, ptr %11, align 4
  %14 = load i32, ptr %12, align 4
  %15 = icmp ne i32 %13, %14
  br label %16

16:                                               ; preds = %2, %6, %10
  %17 = phi i1 [ %15, %10 ], [ false, %6 ], [ false, %2 ]
  ret i1 %17
}

; Function Attrs: nofree nounwind willreturn memory(argmem: read)
declare i32 @memcmp(ptr nocapture, ptr nocapture, i64) #0

attributes #0 = { nofree nounwind willreturn memory(argmem: read) }

the phi value from the entry block turns from true to false
(also the metadata for the loads in the entry block don't need to be dropped?)

will revert this patch

[Allen]

hi @aeubanks, I try to address above metadata issue on D146702.

[Allen]

hi @aeubanks, I recheck your above IR base on alive2, and it seems the transform is right ? see detail https://alive2.llvm.org/ce/z/GMHxjL

[aeubanks]

unless I'm overlooking something, the [ true, %2 ] -> [ false, %2 ] is pretty clearly wrong

[Allen]

In D141188#4253679, @aeubanks wrote:

unless I'm overlooking something, the [ true, %2 ] -> [ false, %2 ] is pretty clearly wrong

Oh, Thanks, you are right. It seems there is a bug in alive2 , so I report it on https://github.com/AliveToolkit/alive2/issues/901

[Allen]

Fix the new added test WebrtcVideoStats, https://alive2.llvm.org/ce/z/EQtb_S

[Allen]

hi @aeubanks, I fixed your case with alive2 proved the new generated IR.

would you help me check if there are any problems with the new version? Thanks

[aeubanks]

I reran the tests that failed before with this patch and now they pass. Feel free to reland, we'll see if any more issues are uncovered after relanding

[Allen]

In D141188#4293445, @aeubanks wrote:

I reran the tests that failed before with this patch and now they pass. Feel free to reland, we'll see if any more issues are uncovered after relanding

Thanks very much

[aeubanks]

another miscompile: https://alive2.llvm.org/ce/z/Zr5dfP

[aeubanks]

(unrelated, but is there any reason this pass is in the codegen pipeline instead of the optimization pipeline?)

[nikic]

In D141188#4303188, @aeubanks wrote:

(unrelated, but is there any reason this pass is in the codegen pipeline instead of the optimization pipeline?)

The last attempt to move MergeICmps and ExpandMemCmp into the optimization pipeline ran into some issue (iirc related to sanitizers) and nobody looked into this in detail since. We would generally prefer to have them in the optimization pipeline, as the current backend position is too late to make full use of subsequent optimizations that may be enabled by them.

[Allen]

In D141188#4302928, @aeubanks wrote:

another miscompile: https://alive2.llvm.org/ce/z/Zr5dfP

Thanks for your report, and it seems the origin eq-chain also has the same issue, see detail https://alive2.llvm.org/ce/z/TDirEG
This is because the logic of memcmp , whose address of the comparison value is from small to large, so this problem needs to be solved independently first.

[Allen]

hi @aeubanks, as the discussion on https://reviews.llvm.org/D149542, the miscompile (https://alive2.llvm.org/ce/z/Zr5dfP) doesn't need to be handled ?

[aeubanks]

In D141188#4320794, @Allen wrote:

hi @aeubanks, as the discussion on https://reviews.llvm.org/D149542, the miscompile (https://alive2.llvm.org/ce/z/Zr5dfP) doesn't need to be handled ?

hmm I swear the miscompile I was seeing wasn't related to poison/freeze, it was similar to the previous miscompile where it was just a wrong true/false. I'll take a look again

[aeubanks]

In D141188#4320968, @aeubanks wrote:

In D141188#4320794, @Allen wrote:

hi @aeubanks, as the discussion on https://reviews.llvm.org/D149542, the miscompile (https://alive2.llvm.org/ce/z/Zr5dfP) doesn't need to be handled ?

hmm I swear the miscompile I was seeing wasn't related to poison/freeze, it was similar to the previous miscompile where it was just a wrong true/false. I'll take a look again

I didn't see an issue reapplying this patch on the test case I was looking at so I've relanded it, sorry for the trouble

[Allen]

Thank you for your careful verification!

[bgraur]

@Allen this last patch is causing a miscompile again.

Here's a small artificial repro:

namespace {
struct Dt {
  Dt() = default;
};

class DtSet {
 public:
  DtSet() = default;
  ~DtSet() = default;

  class iterator {
   public:
    iterator(int a, int b, int idx)
        : a_(a), b_(b), idx_(idx) {}
    bool operator==(const iterator& other) const { return !(*this != other); }
    bool operator!=(const iterator& other) const;
   private:
    int a_, b_;
    int idx_;
  };

  iterator begin() const {
    if (0 == sz_) return end();
    int dt = 0;
    iterator iter(dt, dt+1, 0);
    return iter;
  }
  iterator end() const {
    return iterator(0, 0, sz_);
  }

 private:
  int sz_ = 0;

  DtSet(const DtSet&) = delete;
  DtSet& operator=(const DtSet&) = delete;
};

[[clang::noinline]]
bool DtSet::iterator::operator!=(const iterator& other) const {
  return idx_ != other.idx_ || b_ != other.b_ || a_ != other.a_;
}
}  // namespace

int main() {
  DtSet dt_set;
  if (dt_set.begin() == dt_set.end())
    return 0;
  else
    return 1;
}

Compilation command:

clang  -O3  -std=gnu++17  -pthread   \
   -o /tmp/out  \
   -x c++ /tmp/repro.cc

At this revision the program returns '1' (incorrect), at the previous revision the program returns '0' (as expected).

Could you please revert?

[aeubanks]

In D141188#4327218, @aeubanks wrote:

In D141188#4320968, @aeubanks wrote:

In D141188#4320794, @Allen wrote:

hi @aeubanks, as the discussion on https://reviews.llvm.org/D149542, the miscompile (https://alive2.llvm.org/ce/z/Zr5dfP) doesn't need to be handled ?

hmm I swear the miscompile I was seeing wasn't related to poison/freeze, it was similar to the previous miscompile where it was just a wrong true/false. I'll take a look again

I didn't see an issue reapplying this patch on the test case I was looking at so I've relanded it, sorry for the trouble

Oh that alive2 link is a miscompile even disregarding poison semantics. src is implementing a !=, but it ends up being a == with the memcmp after this pass runs (I got confused with the memcmp return value being 0 on equal instead of 1). Basically it should be
%3 = icmp eq i32 %memcmp, 0 instead of %3 = icmp ne i32 %memcmp, 0.

[Allen]

Thanks for your analysis. The bug is brought by the following code in function mergeComparisons.
As the comparisons is reorded according the memory offset (https://reviews.llvm.org/D149542), so the origin last comparison BB may be not the Comparisons[Comparisons.size() - 1].BB

BasicBlock *const TailBB = Comparisons[Comparisons.size() - 1].BB;

[Allen]

Add new function isLastLinkComparison to check whether the current comparisons is the last comparisons.

[Allen]

In D141188#4343111, @bgraur wrote:

@Allen this last patch is causing a miscompile again.

Here's a small artificial repro:

namespace {
struct Dt {
  Dt() = default;
};

At this revision the program returns '1' (incorrect), at the previous revision the program returns '0' (as expected).

Could you please revert?

I think the above case is fixed with last update, so I'll retry after rebase if nobody object it. sorry for the miscompile.

[Allen]

rebase as precommit test D151091

[bgraur]

@Allen the latest patch is again causing a similar miscompile as the previous one I reported.

We'll work on a reproducer tomorrow. Until then a revert would be greatly appreciated.

[Allen]

In D141188#4393782, @bgraur wrote:

@Allen the latest patch is again causing a similar miscompile as the previous one I reported.

We'll work on a reproducer tomorrow. Until then a revert would be greatly appreciated.

sorry again for the miscompile.
I don't mind to rock back this commit to reduce the interference with your work.

[bgraur]

@Allen would you be so kind to revert the patch while we're working on the reproducer (might take some time)?

[bgraur]

@Allen I got to a repro that requires a headers and two cc files.

The compilation command:

clang -O1  -std=gnu++17  -pthread \
  -x c++ \
  -o /tmp/out \
  impl.cc repro.cc

The reproducer will return 1 (incorrect) at this compiler and 0 (correct) at the version before.

Files are attached here:

impl.h411 BDownload

impl.cc186 BDownload

repro.cc118 BDownload

[Allen]

Thanks for your report , I think the miscompile is related to the splitting of ne-compare-chain.
In fact, it should not be splittted, I mistake this because the alive2 accept the transform, https://alive2.llvm.org/ce/z/iKMekA.
The above transform is invalid as I missing place the related target datalayout information.

target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
target triple = "aarch64-unknown-linux-gnu"

class O {
  ...

  bool EqualsTo(const O &other) const {
    return first_ == other.first_ && type_ == other.type_
      && second_ == other.second_;
  }

 private:
  int type_ = 0;  // Discontinuous layout
  long first_;
  long second_;
};

llvm/lib/Transforms/Scalar/MergeICmps.cpp

if (!Rhs.BaseId)
     return std::nullopt;
   const auto &DL = CmpI->getModule()->getDataLayout();
   return BCECmp(std::move(Lhs), std::move(Rhs),
                 DL.getTypeSizeInBits(CmpI->getOperand(0)->getType()), CmpI);
 }
 
 // Visit the given comparison block. If this is a comparison between two valid
 // BCE atoms, returns the comparison.
-std::optional<BCECmpBlock> visitCmpBlock(Value *const Val,
-                                         BasicBlock *const Block,
-                                         const BasicBlock *const PhiBlock,
-                                         BaseIdentifier &BaseId) {
+std::optional<BCECmpBlock>
+visitCmpBlock(Value *const Baseline, ICmpInst::Predicate &Predicate,
+              Value *const Val, BasicBlock *const Block,
+              const BasicBlock *const PhiBlock, BaseIdentifier &BaseId) {
   if (Block->empty())
     return std::nullopt;
   auto *const BranchI = dyn_cast<BranchInst>(Block->getTerminator());
   if (!BranchI)
     return std::nullopt;
   LLVM_DEBUG(dbgs() << "branch\n");
   Value *Cond;
   ICmpInst::Predicate ExpectedPredicate;
   if (BranchI->isUnconditional()) {
     // In this case, we expect an incoming value which is the result of the
     // comparison. This is the last link in the chain of comparisons (note
     // that this does not mean that this is the last incoming value, blocks
     // can be reordered).
     Cond = Val;
-    ExpectedPredicate = ICmpInst::ICMP_EQ;
+    const auto *const ConstBase = cast<ConstantInt>(Baseline);
+    assert(ConstBase->getType()->isIntegerTy(1) &&
+           "Select condition is not an i1?");
+    ExpectedPredicate =
+        ConstBase->isOne() ? ICmpInst::ICMP_NE : ICmpInst::ICMP_EQ;
+
+    // Remember the correct predicate.
+    Predicate = ExpectedPredicate;
   } else {
+    // All the incoming values must be consistent.
+    if (Baseline != Val)
+      return std::nullopt;
     // In this case, we expect a constant incoming value (the comparison is
     // chained).
     const auto *const Const = cast<ConstantInt>(Val);
+    assert(Const->getType()->isIntegerTy(1) &&
+           "Incoming value is not an i1?");
     LLVM_DEBUG(dbgs() << "const\n");
-    if (!Const->isZero())
+    if (!Const->isZero() && !Const->isOne())
       return std::nullopt;
-    LLVM_DEBUG(dbgs() << "false\n");
+    LLVM_DEBUG(dbgs() << *Const << "\n");
     assert(BranchI->getNumSuccessors() == 2 && "expecting a cond branch");
     BasicBlock *const FalseBlock = BranchI->getSuccessor(1);
     Cond = BranchI->getCondition();
     ExpectedPredicate =
         FalseBlock == PhiBlock ? ICmpInst::ICMP_EQ : ICmpInst::ICMP_NE;
   }
 
   auto *CmpI = dyn_cast<ICmpInst>(Cond);
public:
   }
 
 private:
   PHINode &Phi_;
   // The list of all blocks in the chain, grouped by contiguity.
   std::vector<ContiguousBlocks> MergedBlocks_;
   // The original entry block (before sorting);
   BasicBlock *EntryBlock_;
+  // Remember the predicate type of the chain.
+  ICmpInst::Predicate Predicate_;
 };
 
 static bool areContiguous(const BCECmpBlock &First, const BCECmpBlock &Second) {
   return First.Lhs().BaseId == Second.Lhs().BaseId &&
          First.Rhs().BaseId == Second.Rhs().BaseId &&
          First.Lhs().Offset + First.SizeBits() / 8 == Second.Lhs().Offset &&
          First.Rhs().Offset + First.SizeBits() / 8 == Second.Rhs().Offset;
 }

 
 BCECmpChain::BCECmpChain(const std::vector<BasicBlock *> &Blocks, PHINode &Phi,
                          AliasAnalysis &AA)
     : Phi_(Phi) {
   assert(!Blocks.empty() && "a chain should have at least one block");
   // Now look inside blocks to check for BCE comparisons.
   std::vector<BCECmpBlock> Comparisons;
   BaseIdentifier BaseId;
+  Value *const Baseline = Phi.getIncomingValueForBlock(Blocks[0]);
+  Predicate_ = CmpInst::BAD_ICMP_PREDICATE;
   for (BasicBlock *const Block : Blocks) {
     assert(Block && "invalid block");
-    std::optional<BCECmpBlock> Comparison = visitCmpBlock(
-        Phi.getIncomingValueForBlock(Block), Block, Phi.getParent(), BaseId);
+    std::optional<BCECmpBlock> Comparison =
+        visitCmpBlock(Baseline, Predicate_, Phi.getIncomingValueForBlock(Block),
+                      Block, Phi.getParent(), BaseId);
     if (!Comparison) {
       LLVM_DEBUG(dbgs() << "chain with invalid BCECmpBlock, no merge.\n");
       return;
     }
     if (Comparison->doesOtherWork()) {
       LLVM_DEBUG(dbgs() << "block '" << Comparison->BB->getName()
                         << "' does extra work besides compare\n");
       if (Comparisons.empty()) {

 };
 } // namespace
 
 // Merges the given contiguous comparison blocks into one memcmp block.
 static BasicBlock *mergeComparisons(ArrayRef<BCECmpBlock> Comparisons,
                                     BasicBlock *const InsertBefore,
                                     BasicBlock *const NextCmpBlock,
                                     PHINode &Phi, const TargetLibraryInfo &TLI,
-                                    AliasAnalysis &AA, DomTreeUpdater &DTU) {
+                                    AliasAnalysis &AA, DomTreeUpdater &DTU,
+                                    ICmpInst::Predicate Predicate) {
   assert(!Comparisons.empty() && "merging zero comparisons");
   LLVMContext &Context = NextCmpBlock->getContext();
   const BCECmpBlock &FirstCmp = Comparisons[0];
 
   // Create a new cmp block before next cmp block.
   BasicBlock *const BB =
       BasicBlock::Create(Context, MergedBlockName(Comparisons).Name,
                          NextCmpBlock->getParent(), InsertBefore);
   IRBuilder<> Builder(BB);
   // Add the GEPs from the first BCECmpBlock.
   Value *Lhs, *Rhs;
   if (FirstCmp.Lhs().GEP)
     Lhs = Builder.Insert(FirstCmp.Lhs().GEP->clone());
   else
     Lhs = FirstCmp.Lhs().LoadI->getPointerOperand();
   if (FirstCmp.Rhs().GEP)
     Rhs = Builder.Insert(FirstCmp.Rhs().GEP->clone());
   else
     Rhs = FirstCmp.Rhs().LoadI->getPointerOperand();
 
-  Value *IsEqual = nullptr;
+  Value *ICmpValue = nullptr;
   LLVM_DEBUG(dbgs() << "Merging " << Comparisons.size() << " comparisons -> "
                     << BB->getName() << "\n");
 
   // If there is one block that requires splitting, we do it now, i.e.
   // just before we know we will collapse the chain. The instructions
   // can be executed before any of the instructions in the chain.
   const auto ToSplit = llvm::find_if(
       Comparisons, [](const BCECmpBlock &B) { return B.RequireSplit; });
   if (ToSplit != Comparisons.end()) {
     LLVM_DEBUG(dbgs() << "Splitting non_BCE work to header\n");
     ToSplit->split(BB, AA);
   }
 
   if (Comparisons.size() == 1) {
     LLVM_DEBUG(dbgs() << "Only one comparison, updating branches\n");
     Value *const LhsLoad =
         Builder.CreateLoad(FirstCmp.Lhs().LoadI->getType(), Lhs);
     Value *const RhsLoad =
         Builder.CreateLoad(FirstCmp.Rhs().LoadI->getType(), Rhs);
     // There are no blocks to merge, just do the comparison.
-    IsEqual = Builder.CreateICmpEQ(LhsLoad, RhsLoad);
+    ICmpValue = Builder.CreateICmp(Predicate, LhsLoad, RhsLoad);
   } else {
     const unsigned TotalSizeBits = std::accumulate(
         Comparisons.begin(), Comparisons.end(), 0u,
         [](int Size, const BCECmpBlock &C) { return Size + C.SizeBits(); });
 
     // memcmp expects a 'size_t' argument and returns 'int'.
     unsigned SizeTBits = TLI.getSizeTSize(*Phi.getModule());
     unsigned IntBits = TLI.getIntSize();
 
     // Create memcmp() == 0.
     const auto &DL = Phi.getModule()->getDataLayout();
     Value *const MemCmpCall = emitMemCmp(
         Lhs, Rhs,
         ConstantInt::get(Builder.getIntNTy(SizeTBits), TotalSizeBits / 8),
         Builder, DL, &TLI);
-    IsEqual = Builder.CreateICmpEQ(
-        MemCmpCall, ConstantInt::get(Builder.getIntNTy(IntBits), 0));
+    ICmpValue = Builder.CreateICmp(
+        Predicate, MemCmpCall, ConstantInt::get(Builder.getIntNTy(IntBits), 0));
   }
 
   BasicBlock *const PhiBB = Phi.getParent();
   // Add a branch to the next basic block in the chain.
   if (NextCmpBlock == PhiBB) {
     // Continue to phi, passing it the comparison result.
     Builder.CreateBr(PhiBB);
-    Phi.addIncoming(IsEqual, BB);
+    Phi.addIncoming(ICmpValue, BB);
     DTU.applyUpdates({{DominatorTree::Insert, BB, PhiBB}});
   } else {
     // Continue to next block if equal, exit to phi else.
-    Builder.CreateCondBr(IsEqual, NextCmpBlock, PhiBB);
+    Builder.CreateCondBr(ICmpValue, NextCmpBlock, PhiBB);
     Phi.addIncoming(ConstantInt::getFalse(Context), BB);
     DTU.applyUpdates({{DominatorTree::Insert, BB, NextCmpBlock},
                       {DominatorTree::Insert, BB, PhiBB}});
   }
   return BB;
 }
 
 bool BCECmpChain::simplify(const TargetLibraryInfo &TLI, AliasAnalysis &AA,
                            DomTreeUpdater &DTU) {
   assert(atLeastOneMerged() && "simplifying trivial BCECmpChain");
   LLVM_DEBUG(dbgs() << "Simplifying comparison chain starting at block "
                     << EntryBlock_->getName() << "\n");
 
   // Effectively merge blocks. We go in the reverse direction from the phi block
   // so that the next block is always available to branch to.
   BasicBlock *InsertBefore = EntryBlock_;
   BasicBlock *NextCmpBlock = Phi_.getParent();
+  assert(Predicate_ != CmpInst::BAD_ICMP_PREDICATE &&
+         "Got the chain of comparisons");
   for (const auto &Blocks : reverse(MergedBlocks_)) {
     InsertBefore = NextCmpBlock = mergeComparisons(
-        Blocks, InsertBefore, NextCmpBlock, Phi_, TLI, AA, DTU);
+        Blocks, InsertBefore, NextCmpBlock, Phi_, TLI, AA, DTU, Predicate_);
   }
 
   // Replace the original cmp chain with the new cmp chain by pointing all
   // predecessors of EntryBlock_ to NextCmpBlock instead. This makes all cmp
   // blocks in the old chain unreachable.
   while (!pred_empty(EntryBlock_)) {
     BasicBlock* const Pred = *pred_begin(EntryBlock_);
     LLVM_DEBUG(dbgs() << "Updating jump into old chain from " << Pred->getName()
if (!isa<ICmpInst>(Phi.getIncomingValue(I)) ||
       return false;
     }
     LastBlock = Phi.getIncomingBlock(I);
   }
   if (!LastBlock) {
     // There is no non-constant block.
     LLVM_DEBUG(dbgs() << "skip: no non-constant block\n");
     return false;
   }
   if (LastBlock->getSingleSuccessor() != Phi.getParent()) {
     LLVM_DEBUG(dbgs() << "skip: last block non-phi successor\n");
     return false;
   }
 
   const auto Blocks =
       getOrderedBlocks(Phi, LastBlock, Phi.getNumIncomingValues());
   if (Blocks.empty()) return false;

llvm/test/Transforms/MergeICmps/X86/pr59740.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -passes=mergeicmps -verify-dom-info -S -mtriple=x86_64-unknown-unknown | FileCheck %s
+
+%struct.S = type { i8, i8, i8, i8 }
+
+define noundef i1 @_Z2neR1SS0_(ptr nocapture readonly align 1 dereferenceable(4) %s0, ptr nocapture readonly align 1 dereferenceable(4) %s1) {
+; CHECK-LABEL: @_Z2neR1SS0_(
+; CHECK-NEXT:  "bb0+bb1+bb2+bb3":
+; CHECK-NEXT:    [[MEMCMP:%.*]] = call i32 @memcmp(ptr [[S0:%.*]], ptr [[S1:%.*]], i64 4)
+; CHECK-NEXT:    [[TMP0:%.*]] = icmp ne i32 [[MEMCMP]], 0
+; CHECK-NEXT:    br label [[BB4:%.*]]
+; CHECK:       bb4:
+; CHECK-NEXT:    ret i1 [[TMP0]]
+;
+bb0:
+  %v0 = load i8, ptr %s0, align 1
+  %v1 = load i8, ptr %s1, align 1
+  %cmp0 = icmp eq i8 %v0, %v1
+  br i1 %cmp0, label %bb1, label %bb4
+
+bb1:                                              ; preds = %bb0
+  %s2 = getelementptr inbounds %struct.S, ptr %s0, i64 0, i32 1
+  %v2 = load i8, ptr %s2, align 1
+  %s3 = getelementptr inbounds %struct.S, ptr %s1, i64 0, i32 1
+  %v3 = load i8, ptr %s3, align 1
+  %cmp1 = icmp eq i8 %v2, %v3
+  br i1 %cmp1, label %bb2, label %bb4
+
+bb2:                                             ; preds = %bb1
+  %s4 = getelementptr inbounds %struct.S, ptr %s0, i64 0, i32 2
+  %v4 = load i8, ptr %s4, align 1
+  %s5 = getelementptr inbounds %struct.S, ptr %s1, i64 0, i32 2
+  %v5 = load i8, ptr %s5, align 1
+  %cmp2 = icmp eq i8 %v4, %v5
+  br i1 %cmp2, label %bb3, label %bb4
+
+bb3:                                               ; preds = %bb2
+  %s6 = getelementptr inbounds %struct.S, ptr %s0, i64 0, i32 3
+  %v6 = load i8, ptr %s6, align 1
+  %s7 = getelementptr inbounds %struct.S, ptr %s1, i64 0, i32 3
+  %v7 = load i8, ptr %s7, align 1
+  %cmp3 = icmp ne i8 %v6, %v7
+  br label %bb4
+
+bb4:                                               ; preds = %bb0, %bb1, %bb2, %bb3
+  %cmp = phi i1 [ true, %bb0 ], [ true, %bb1 ], [ true, %bb2 ], [ %cmp3, %bb3 ]
+  ret i1 %cmp
+}
+
+; Negative test: Incorrect const value in PHI node
+define noundef i1 @cmp_ne_incorrect_const(ptr nocapture readonly align 1 dereferenceable(4) %s0, ptr nocapture readonly align 1 dereferenceable(4) %s1) {
+; CHECK-LABEL: @cmp_ne_incorrect_const(
+; CHECK-NEXT:  bb0:
+; CHECK-NEXT:    [[V0:%.*]] = load i8, ptr [[S0:%.*]], align 1
+; CHECK-NEXT:    [[V1:%.*]] = load i8, ptr [[S1:%.*]], align 1
+; CHECK-NEXT:    [[CMP0:%.*]] = icmp eq i8 [[V0]], [[V1]]
+; CHECK-NEXT:    br i1 [[CMP0]], label [[BB1:%.*]], label [[BB2:%.*]]
+; CHECK:       bb1:
+; CHECK-NEXT:    [[S6:%.*]] = getelementptr inbounds [[STRUCT_S:%.*]], ptr [[S0]], i64 0, i32 1
+; CHECK-NEXT:    [[V6:%.*]] = load i8, ptr [[S6]], align 1
+; CHECK-NEXT:    [[S7:%.*]] = getelementptr inbounds [[STRUCT_S]], ptr [[S1]], i64 0, i32 1
+; CHECK-NEXT:    [[V7:%.*]] = load i8, ptr [[S7]], align 1
+; CHECK-NEXT:    [[CMP3:%.*]] = icmp ne i8 [[V6]], [[V7]]
+; CHECK-NEXT:    br label [[BB2]]
+; CHECK:       bb2:
+; CHECK-NEXT:    [[CMP:%.*]] = phi i1 [ false, [[BB0:%.*]] ], [ [[CMP3]], [[BB1]] ]
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+bb0:
+  %v0 = load i8, ptr %s0, align 1
+  %v1 = load i8, ptr %s1, align 1
+  %cmp0 = icmp eq i8 %v0, %v1
+  br i1 %cmp0, label %bb1, label %bb2
+
+bb1:                                               ; preds = %bb0
+  %s6 = getelementptr inbounds %struct.S, ptr %s0, i64 0, i32 1
+  %v6 = load i8, ptr %s6, align 1
+  %s7 = getelementptr inbounds %struct.S, ptr %s1, i64 0, i32 1
+  %v7 = load i8, ptr %s7, align 1
+  %cmp3 = icmp ne i8 %v6, %v7
+  br label %bb2
+
+bb2:                                               ; preds = %bb0, %bb1
+  %cmp = phi i1 [ false, %bb0 ], [ %cmp3, %bb1 ]
+  ret i1 %cmp
+}