forked from llvm/llvm-project
-
Notifications
You must be signed in to change notification settings - Fork 98
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[InstCombine] Fix infinite loop due to bitcast <-> phi transforms (#34)
* [InstCombine] Add test for iterator invalidation bug; NFC * [InstCombine] Fix user iterator invalidation in bitcast of phi transform This fixes the issue encountered in D71164. Instead of using a range-based for, manually iterate over the users and advance the iterator beforehand, so we do not skip any users due to iterator invalidation. Differential Revision: https://reviews.llvm.org/D72657 * [InstCombine] Make combineLoadToNewType a method; NFC So it can be reused as part of other combines. In particular for D71164. Conflicts: llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp * [InstCombine] Fix infinite loop due to bitcast <-> phi transforms Fix for https://bugs.llvm.org/show_bug.cgi?id=44245. The optimizeBitCastFromPhi() and FoldPHIArgOpIntoPHI() end up fighting against each other, because optimizeBitCastFromPhi() assumes that bitcasts of loads will get folded. This doesn't happen here, because a dangling phi node prevents the one-use fold in https://github.com/llvm/llvm-project/blob/master/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp#L620-L628 from triggering. This patch fixes the issue by explicitly performing the load combine as part of the bitcast of phi transform. Other attempts to force the load to be combined first were ultimately too unreliable. Differential Revision: https://reviews.llvm.org/D71164 * [InstCombine] Improve infinite loop detection Summary: This patch limits the default number of iterations performed by InstCombine. It also exposes a new option that allows to specify how many iterations is considered getting stuck in an infinite loop. Based on experiments performed on real-world C++ programs, InstCombine seems to perform at most ~8-20 iterations, so treating 1000 iterations as an infinite loop seems like a safe choice. See D71145 for details. The two limits can be specified via command line options. Reviewers: spatel, lebedev.ri, nikic, xbolva00, grosser Reviewed By: spatel Subscribers: hiraditya, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D71673 * [InstCombine] Add tests for PR44242 Differential Revision: https://reviews.llvm.org/D71260 * [InstCombine] Don't rewrite phi-of-bitcast when the phi has other users Judging by the existing comments, this was the intention, but the transform never actually checked if the existing phi's would be removed. See https://bugs.llvm.org/show_bug.cgi?id=44242 for an example where this causes much worse code generation on AMDGPU. Differential Revision: https://reviews.llvm.org/D71209 Co-authored-by: Nikita Popov <nikita.ppv@googlemail.com> Co-authored-by: Jakub Kuderski <kubakuderski@gmail.com> Co-authored-by: Connor Abbott <connor@abbott.cx>
- Loading branch information
1 parent
6071408
commit a6f4c1b
Showing
7 changed files
with
503 additions
and
33 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
33 changes: 33 additions & 0 deletions
33
llvm/test/Transforms/InstCombine/bitcast-phi-uselistorder.ll
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py | ||
; RUN: opt -S -instcombine < %s | FileCheck %s | ||
|
||
@Q = internal unnamed_addr global double 1.000000e+00, align 8 | ||
|
||
define double @test(i1 %c, i64* %p) { | ||
; CHECK-LABEL: @test( | ||
; CHECK-NEXT: entry: | ||
; CHECK-NEXT: br i1 [[C:%.*]], label [[IF:%.*]], label [[END:%.*]] | ||
; CHECK: if: | ||
; CHECK-NEXT: [[LOAD1:%.*]] = load double, double* @Q, align 8 | ||
; CHECK-NEXT: br label [[END]] | ||
; CHECK: end: | ||
; CHECK-NEXT: [[TMP0:%.*]] = phi double [ 0.000000e+00, [[ENTRY:%.*]] ], [ [[LOAD1]], [[IF]] ] | ||
; CHECK-NEXT: [[TMP1:%.*]] = bitcast i64* [[P:%.*]] to double* | ||
; CHECK-NEXT: store double [[TMP0]], double* [[TMP1]], align 8 | ||
; CHECK-NEXT: ret double [[TMP0]] | ||
; | ||
entry: | ||
br i1 %c, label %if, label %end | ||
|
||
if: | ||
%load = load i64, i64* bitcast (double* @Q to i64*), align 8 | ||
br label %end | ||
|
||
end: | ||
%phi = phi i64 [ 0, %entry ], [ %load, %if ] | ||
store i64 %phi, i64* %p, align 8 | ||
%cast = bitcast i64 %phi to double | ||
ret double %cast | ||
|
||
uselistorder i64 %phi, { 1, 0 } | ||
} |
Oops, something went wrong.