Skip to content

Commit

Permalink
[DAGCombiner] Pre-commit test case for miscompile bug in combineShift…
Browse files Browse the repository at this point in the history
…OfShiftedLogic

DAGCombiner is trying to fold shl over binops, and in the process
combining it with another shl. However it needs to be more careful
to ensure that the sum of the shift counts fits in the type used
for the shift amount.
For example, X86 is using i8 as shift amount type. So we need to
make sure that the sum of the shift amounts isn't greater than 255.

Fix will be applied in a later commit. This only pre-commits the
test case to show that we currently get the wrong result.

Bug was found when testing the C23 BitInt feature.
  • Loading branch information
bjope committed Apr 23, 2024
1 parent 0661af8 commit 5fd9bbd
Showing 1 changed file with 65 additions and 0 deletions.
65 changes: 65 additions & 0 deletions llvm/test/CodeGen/X86/shift-combine.ll
Original file line number Diff line number Diff line change
Expand Up @@ -787,3 +787,68 @@ define <4 x i32> @or_tree_with_mismatching_shifts_vec_i32(<4 x i32> %a, <4 x i32
%r = or <4 x i32> %or.ab, %or.cd
ret <4 x i32> %r
}

; FIXME: Reproducer for a DAGCombiner::combineShiftOfShiftedLogic
; bug. DAGCombiner need to check that the sum of the shift amounts fits in i8,
; which is the legal type used to described X86 shift amounts. Verify that we
; do not try to create a shift with 130+160 as shift amount, and verify that
; the stored value do not depend on %a1.
define void @combineShiftOfShiftedLogic(i128 %a1, i32 %a2, ptr %p) {
; X86-LABEL: combineShiftOfShiftedLogic:
; X86: # %bb.0:
; X86-NEXT: pushl %ebx
; X86-NEXT: .cfi_def_cfa_offset 8
; X86-NEXT: pushl %edi
; X86-NEXT: .cfi_def_cfa_offset 12
; X86-NEXT: pushl %esi
; X86-NEXT: .cfi_def_cfa_offset 16
; X86-NEXT: .cfi_offset %esi, -16
; X86-NEXT: .cfi_offset %edi, -12
; X86-NEXT: .cfi_offset %ebx, -8
; X86-NEXT: movl {{[0-9]+}}(%esp), %eax
; X86-NEXT: movl {{[0-9]+}}(%esp), %ecx
; X86-NEXT: movl {{[0-9]+}}(%esp), %edx
; X86-NEXT: movl {{[0-9]+}}(%esp), %ebx
; X86-NEXT: movl {{[0-9]+}}(%esp), %edi
; X86-NEXT: movl %edi, %esi
; X86-NEXT: shldl $2, %ebx, %edi
; X86-NEXT: shldl $2, %edx, %ebx
; X86-NEXT: shrl $30, %esi
; X86-NEXT: orl {{[0-9]+}}(%esp), %esi
; X86-NEXT: shldl $2, %ecx, %edx
; X86-NEXT: shll $2, %ecx
; X86-NEXT: movl %edi, 16(%eax)
; X86-NEXT: movl %ebx, 12(%eax)
; X86-NEXT: movl %edx, 8(%eax)
; X86-NEXT: movl %ecx, 4(%eax)
; X86-NEXT: movl %esi, 20(%eax)
; X86-NEXT: movl $0, (%eax)
; X86-NEXT: popl %esi
; X86-NEXT: .cfi_def_cfa_offset 12
; X86-NEXT: popl %edi
; X86-NEXT: .cfi_def_cfa_offset 8
; X86-NEXT: popl %ebx
; X86-NEXT: .cfi_def_cfa_offset 4
; X86-NEXT: retl
;
; X64-LABEL: combineShiftOfShiftedLogic:
; X64: # %bb.0:
; X64-NEXT: # kill: def $edx killed $edx def $rdx
; X64-NEXT: shlq $32, %rdx
; X64-NEXT: movq %rsi, %rax
; X64-NEXT: shrq $30, %rax
; X64-NEXT: orq %rdx, %rax
; X64-NEXT: shldq $34, %rdi, %rsi
; X64-NEXT: shlq $34, %rdi
; X64-NEXT: movq %rsi, 8(%rcx)
; X64-NEXT: movq %rdi, (%rcx)
; X64-NEXT: movq %rax, 16(%rcx)
; X64-NEXT: retq
%zext1 = zext i128 %a1 to i192
%zext2 = zext i32 %a2 to i192
%shl = shl i192 %zext1, 130
%or = or i192 %shl, %zext2
%res = shl i192 %or, 160
store i192 %res, ptr %p, align 8
ret void
}

0 comments on commit 5fd9bbd

Please sign in to comment.