Skip to content

Commit 5d111a2

Browse files
authored
[DAGCombiner] Avoid double deletion when replacing multiple frozen/unfrozen uses (#155427)
Closes #155345. In the original case, we have one frozen use and two unfrozen uses: ``` t73: i8 = select t81, Constant:i8<0>, t18 t75: i8 = select t10, t18, t73 t59: i8 = freeze t18 (combining) t80: i8 = freeze t59 (another user of t59) ``` In `DAGCombiner::visitFREEZE`, we replace all uses of `t18` with `t59`. After updating the uses, `t59: i8 = freeze t18` will be updated to `t59: i8 = freeze t59` (`AddModifiedNodeToCSEMaps`) and CSEed into `t80: i8 = freeze t59` (`ReplaceAllUsesWith`). As the previous call to `AddModifiedNodeToCSEMaps` already removed `t59` from the CSE map, `ReplaceAllUsesWith` cannot remove `t59` again. For clarity, see the following call graph: ``` ReplaceAllUsesOfValueWith(t18, t59) ReplaceAllUsesWith(t18, t59) RemoveNodeFromCSEMaps(t73) update t73 AddModifiedNodeToCSEMaps(t73) RemoveNodeFromCSEMaps(t75) update t75 AddModifiedNodeToCSEMaps(t75) RemoveNodeFromCSEMaps(t59) <- first delection update t59 AddModifiedNodeToCSEMaps(t59) ReplaceAllUsesWith(t59, t80) RemoveNodeFromCSEMaps(t59) <- second delection Boom! ``` This patch unfreezes all the uses first to avoid triggering CSE when introducing cycles.
1 parent fe78a9a commit 5d111a2

File tree

3 files changed

+185
-138
lines changed

3 files changed

+185
-138
lines changed

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16796,6 +16796,8 @@ SDValue DAGCombiner::visitFREEZE(SDNode *N) {
1679616796
// If we have frozen and unfrozen users of N0, update so everything uses N.
1679716797
if (!N0.isUndef() && !N0.hasOneUse()) {
1679816798
SDValue FrozenN0(N, 0);
16799+
// Unfreeze all uses of N to avoid double deleting N from the CSE map.
16800+
DAG.ReplaceAllUsesOfValueWith(FrozenN0, N0);
1679916801
DAG.ReplaceAllUsesOfValueWith(N0, FrozenN0);
1680016802
// ReplaceAllUsesOfValueWith will have also updated the use in N, thus
1680116803
// creating a cycle in a DAG. Let's undo that by mutating the freeze.

llvm/test/CodeGen/X86/freeze.ll

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,3 +141,48 @@ entry:
141141
%z = urem i32 %y, 10
142142
ret i32 %z
143143
}
144+
145+
; Make sure we don't crash when replacing all uses of N with an existing freeze N.
146+
147+
define i64 @pr155345(ptr %p1, i1 %cond, ptr %p2, ptr %p3) {
148+
; X86ASM-LABEL: pr155345:
149+
; X86ASM: # %bb.0: # %entry
150+
; X86ASM-NEXT: movzbl (%rdi), %edi
151+
; X86ASM-NEXT: xorl %eax, %eax
152+
; X86ASM-NEXT: orb $1, %dil
153+
; X86ASM-NEXT: movb %dil, (%rdx)
154+
; X86ASM-NEXT: movzbl %dil, %edx
155+
; X86ASM-NEXT: cmovel %edx, %eax
156+
; X86ASM-NEXT: sete %dil
157+
; X86ASM-NEXT: testb $1, %sil
158+
; X86ASM-NEXT: cmovnel %edx, %eax
159+
; X86ASM-NEXT: movb %dl, (%rcx)
160+
; X86ASM-NEXT: movl $1, %edx
161+
; X86ASM-NEXT: movl %eax, %ecx
162+
; X86ASM-NEXT: shlq %cl, %rdx
163+
; X86ASM-NEXT: orb %sil, %dil
164+
; X86ASM-NEXT: movzbl %dil, %eax
165+
; X86ASM-NEXT: andl %edx, %eax
166+
; X86ASM-NEXT: andl $1, %eax
167+
; X86ASM-NEXT: retq
168+
entry:
169+
%load1 = load i8, ptr %p1, align 1
170+
%v1 = or i8 %load1, 1
171+
%v2 = zext i8 %v1 to i32
172+
store i8 %v1, ptr %p2, align 1
173+
%v3 = load i8, ptr %p2, align 1
174+
%ext1 = sext i8 %v3 to i64
175+
%ext2 = zext i32 %v2 to i64
176+
%cmp1 = icmp ult i64 0, %ext1
177+
%v4 = select i1 %cond, i1 false, i1 %cmp1
178+
%sel1 = select i1 %v4, i64 0, i64 %ext2
179+
%shl = shl i64 1, %sel1
180+
store i8 %v1, ptr %p3, align 1
181+
%v5 = load i8, ptr %p3, align 1
182+
%ext3 = sext i8 %v5 to i64
183+
%cmp2 = icmp ult i64 0, %ext3
184+
%v6 = select i1 %cond, i1 false, i1 %cmp2
185+
%sel2 = select i1 %v6, i64 0, i64 1
186+
%and = and i64 %sel2, %shl
187+
ret i64 %and
188+
}

0 commit comments

Comments
 (0)