Skip to content

[InstCombine] Don't check uses of constant exprs #113684

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 28, 2024

Conversation

dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Oct 25, 2024

This patch skips constant expressions to avoid iterating over uses on other functions.

Fix crash reported in #105510 (comment).

@llvmbot
Copy link
Member

llvmbot commented Oct 25, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

Address comment #105510 (comment).


Full diff: https://github.com/llvm/llvm-project/pull/113684.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstructionCombining.cpp (+1-1)
  • (added) llvm/test/Transforms/InstCombine/pr105510.ll (+43)
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index c8b9f166b16020..60be087430be0a 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -3753,7 +3753,7 @@ Instruction *InstCombinerImpl::visitBranchInst(BranchInst &BI) {
   }
 
   // Replace all dominated uses of the condition with true/false
-  if (BI.getSuccessor(0) != BI.getSuccessor(1)) {
+  if (!isa<Constant>(Cond) && BI.getSuccessor(0) != BI.getSuccessor(1)) {
     for (auto &U : make_early_inc_range(Cond->uses())) {
       BasicBlockEdge Edge0(BI.getParent(), BI.getSuccessor(0));
       if (DT.dominates(Edge0, U)) {
diff --git a/llvm/test/Transforms/InstCombine/pr105510.ll b/llvm/test/Transforms/InstCombine/pr105510.ll
new file mode 100644
index 00000000000000..844fa14ad991ee
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/pr105510.ll
@@ -0,0 +1,43 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -S -passes=instcombine < %s | FileCheck %s
+
+; Make sure we don't crash in this case.
+@g = global i32 0
+
+define i1 @foo() {
+; CHECK-LABEL: define i1 @foo() {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    br i1 ptrtoint (ptr @g to i1), label %[[IF_THEN:.*]], label %[[IF_ELSE:.*]]
+; CHECK:       [[IF_THEN]]:
+; CHECK-NEXT:    ret i1 true
+; CHECK:       [[IF_ELSE]]:
+; CHECK-NEXT:    ret i1 false
+;
+entry:
+  br i1 ptrtoint (ptr @g to i1), label %if.then, label %if.else
+
+if.then:
+  ret i1 true
+
+if.else:
+  ret i1 false
+}
+
+define i1 @bar() {
+; CHECK-LABEL: define i1 @bar() {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    br i1 ptrtoint (ptr @g to i1), label %[[IF_THEN:.*]], label %[[IF_ELSE:.*]]
+; CHECK:       [[IF_THEN]]:
+; CHECK-NEXT:    ret i1 true
+; CHECK:       [[IF_ELSE]]:
+; CHECK-NEXT:    ret i1 false
+;
+entry:
+  br i1 ptrtoint (ptr @g to i1), label %if.then, label %if.else
+
+if.then:
+  ret i1 true
+
+if.else:
+  ret i1 false
+}

Copy link
Collaborator

@mikaelholmen mikaelholmen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've verified that this solves the problem I saw.

Looks good to mne, but I'm not very familiar with this so please wait for someone else to accept it.

Thanks!

@mikaelholmen
Copy link
Collaborator

I'm happy with this. Thanks again!

Copy link
Contributor

@goldsteinn goldsteinn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM although I think the commit message is extremely vague.

@dtcxzyw dtcxzyw merged commit 5155c38 into llvm:main Oct 28, 2024
8 checks passed
@dtcxzyw dtcxzyw deleted the fix-105510-crash branch October 28, 2024 07:09
@llvm-ci
Copy link
Collaborator

llvm-ci commented Oct 28, 2024

LLVM Buildbot has detected a new failure on builder sanitizer-x86_64-linux-bootstrap-asan running on sanitizer-buildbot1 while building llvm at step 2 "annotate".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/52/builds/3270

Here is the relevant piece of the build log for the reference
Step 2 (annotate) failure: 'python ../sanitizer_buildbot/sanitizers/zorg/buildbot/builders/sanitizers/buildbot_selector.py' (failure)
...
llvm-lit: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:506: note: using lld-link: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/lld-link
llvm-lit: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:506: note: using ld64.lld: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/ld64.lld
llvm-lit: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:506: note: using wasm-ld: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/wasm-ld
llvm-lit: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:506: note: using ld.lld: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/ld.lld
llvm-lit: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:506: note: using lld-link: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/lld-link
llvm-lit: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:506: note: using ld64.lld: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/ld64.lld
llvm-lit: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:506: note: using wasm-ld: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/wasm-ld
llvm-lit: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/utils/lit/lit/main.py:72: note: The test suite configuration requested an individual test timeout of 0 seconds but a timeout of 900 seconds was requested on the command line. Forcing timeout to be 900 seconds.
-- Testing: 86550 of 86551 tests, 88 workers --
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.
FAIL: lld :: COFF/duplicate-absolute-same.s (83623 of 86550)
******************** TEST 'lld :: COFF/duplicate-absolute-same.s' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 2: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/llvm-mc -triple x86_64-windows-msvc -filetype obj -o /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/tools/lld/test/COFF/Output/duplicate-absolute-same.s.tmp.obj /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/lld/test/COFF/duplicate-absolute-same.s
+ /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/llvm-mc -triple x86_64-windows-msvc -filetype obj -o /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/tools/lld/test/COFF/Output/duplicate-absolute-same.s.tmp.obj /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/lld/test/COFF/duplicate-absolute-same.s
RUN: at line 3: echo -e ".globl myabsolute\nmyabsolute = 0" > /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/tools/lld/test/COFF/Output/duplicate-absolute-same.s.tmp.dupl.s
+ echo -e '.globl myabsolute\nmyabsolute = 0'
RUN: at line 4: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/llvm-mc -triple x86_64-windows-msvc -filetype obj -o /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/tools/lld/test/COFF/Output/duplicate-absolute-same.s.tmp.dupl.obj /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/tools/lld/test/COFF/Output/duplicate-absolute-same.s.tmp.dupl.s
+ /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/llvm-mc -triple x86_64-windows-msvc -filetype obj -o /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/tools/lld/test/COFF/Output/duplicate-absolute-same.s.tmp.dupl.obj /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/tools/lld/test/COFF/Output/duplicate-absolute-same.s.tmp.dupl.s
RUN: at line 5: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/lld-link /out:/home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/tools/lld/test/COFF/Output/duplicate-absolute-same.s.tmp.exe /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/tools/lld/test/COFF/Output/duplicate-absolute-same.s.tmp.obj /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/tools/lld/test/COFF/Output/duplicate-absolute-same.s.tmp.dupl.obj -subsystem:console -entry:entry 2>&1 | /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/count 0
+ /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/count 0
+ /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/lld-link /out:/home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/tools/lld/test/COFF/Output/duplicate-absolute-same.s.tmp.exe /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/tools/lld/test/COFF/Output/duplicate-absolute-same.s.tmp.obj /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/tools/lld/test/COFF/Output/duplicate-absolute-same.s.tmp.dupl.obj -subsystem:console -entry:entry
Expected 0 lines, got 1.

--

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
Slowest Tests:
--------------------------------------------------------------------------
240.70s: LLVM :: CodeGen/AMDGPU/sched-group-barrier-pipeline-solver.mir
125.70s: Clang :: Driver/fsanitize.c
122.82s: Clang :: Preprocessor/riscv-target-features.c
105.11s: Clang :: OpenMP/target_defaultmap_codegen_01.cpp
101.22s: Clang :: OpenMP/target_update_codegen.cpp
98.86s: Clang :: Driver/arm-cortex-cpus-2.c
97.00s: Clang :: Driver/arm-cortex-cpus-1.c
80.11s: Clang :: Preprocessor/aarch64-target-features.c
79.89s: Clang :: Preprocessor/arm-target-features.c
71.75s: Clang :: Preprocessor/predefined-arch-macros.c
70.52s: Clang :: Driver/clang_f_opts.c
69.22s: Clang :: Driver/linux-ld.c
68.63s: LLVM :: CodeGen/RISCV/attributes.ll
66.61s: Clang :: Analysis/a_flaky_crash.cpp
63.01s: Clang :: Driver/cl-options.c
58.27s: Clang :: Driver/x86-target-features.c
Step 10 (stage2/asan check) failure: stage2/asan check (failure)
...
llvm-lit: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:506: note: using lld-link: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/lld-link
llvm-lit: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:506: note: using ld64.lld: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/ld64.lld
llvm-lit: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:506: note: using wasm-ld: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/wasm-ld
llvm-lit: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:506: note: using ld.lld: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/ld.lld
llvm-lit: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:506: note: using lld-link: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/lld-link
llvm-lit: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:506: note: using ld64.lld: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/ld64.lld
llvm-lit: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:506: note: using wasm-ld: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/wasm-ld
llvm-lit: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/utils/lit/lit/main.py:72: note: The test suite configuration requested an individual test timeout of 0 seconds but a timeout of 900 seconds was requested on the command line. Forcing timeout to be 900 seconds.
-- Testing: 86550 of 86551 tests, 88 workers --
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.
FAIL: lld :: COFF/duplicate-absolute-same.s (83623 of 86550)
******************** TEST 'lld :: COFF/duplicate-absolute-same.s' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 2: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/llvm-mc -triple x86_64-windows-msvc -filetype obj -o /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/tools/lld/test/COFF/Output/duplicate-absolute-same.s.tmp.obj /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/lld/test/COFF/duplicate-absolute-same.s
+ /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/llvm-mc -triple x86_64-windows-msvc -filetype obj -o /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/tools/lld/test/COFF/Output/duplicate-absolute-same.s.tmp.obj /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/lld/test/COFF/duplicate-absolute-same.s
RUN: at line 3: echo -e ".globl myabsolute\nmyabsolute = 0" > /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/tools/lld/test/COFF/Output/duplicate-absolute-same.s.tmp.dupl.s
+ echo -e '.globl myabsolute\nmyabsolute = 0'
RUN: at line 4: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/llvm-mc -triple x86_64-windows-msvc -filetype obj -o /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/tools/lld/test/COFF/Output/duplicate-absolute-same.s.tmp.dupl.obj /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/tools/lld/test/COFF/Output/duplicate-absolute-same.s.tmp.dupl.s
+ /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/llvm-mc -triple x86_64-windows-msvc -filetype obj -o /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/tools/lld/test/COFF/Output/duplicate-absolute-same.s.tmp.dupl.obj /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/tools/lld/test/COFF/Output/duplicate-absolute-same.s.tmp.dupl.s
RUN: at line 5: /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/lld-link /out:/home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/tools/lld/test/COFF/Output/duplicate-absolute-same.s.tmp.exe /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/tools/lld/test/COFF/Output/duplicate-absolute-same.s.tmp.obj /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/tools/lld/test/COFF/Output/duplicate-absolute-same.s.tmp.dupl.obj -subsystem:console -entry:entry 2>&1 | /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/count 0
+ /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/count 0
+ /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/lld-link /out:/home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/tools/lld/test/COFF/Output/duplicate-absolute-same.s.tmp.exe /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/tools/lld/test/COFF/Output/duplicate-absolute-same.s.tmp.obj /home/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/tools/lld/test/COFF/Output/duplicate-absolute-same.s.tmp.dupl.obj -subsystem:console -entry:entry
Expected 0 lines, got 1.

--

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
Slowest Tests:
--------------------------------------------------------------------------
240.70s: LLVM :: CodeGen/AMDGPU/sched-group-barrier-pipeline-solver.mir
125.70s: Clang :: Driver/fsanitize.c
122.82s: Clang :: Preprocessor/riscv-target-features.c
105.11s: Clang :: OpenMP/target_defaultmap_codegen_01.cpp
101.22s: Clang :: OpenMP/target_update_codegen.cpp
98.86s: Clang :: Driver/arm-cortex-cpus-2.c
97.00s: Clang :: Driver/arm-cortex-cpus-1.c
80.11s: Clang :: Preprocessor/aarch64-target-features.c
79.89s: Clang :: Preprocessor/arm-target-features.c
71.75s: Clang :: Preprocessor/predefined-arch-macros.c
70.52s: Clang :: Driver/clang_f_opts.c
69.22s: Clang :: Driver/linux-ld.c
68.63s: LLVM :: CodeGen/RISCV/attributes.ll
66.61s: Clang :: Analysis/a_flaky_crash.cpp
63.01s: Clang :: Driver/cl-options.c
58.27s: Clang :: Driver/x86-target-features.c

NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
This patch skips constant expressions to avoid iterating over uses on
other functions.

Fix crash reported in
llvm#105510 (comment).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants