Skip to content
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.

Sign up for GitHub

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

Jump to bottom

miscompilation due to LoopVectorize making a function vulnerable to integer divide-by-zero #89958

Open
regehr opened this issue Apr 24, 2024 · 1 comment · May be fixed by #92177
Open

miscompilation due to LoopVectorize making a function vulnerable to integer divide-by-zero #89958

regehr opened this issue Apr 24, 2024 · 1 comment · May be fixed by #92177
Assignees
@fhahn
Labels
loopoptim miscompilation vectorization

Comments

@regehr
Copy link
Contributor

regehr commented Apr 24, 2024

https://alive2.llvm.org/ce/z/Kx2anL

this function:

define i64 @f(i64 %0) {
  br label %2

2:                                                ; preds = %5, %1
  %3 = phi i64 [ 0, %1 ], [ %6, %5 ]
  %4 = icmp slt i64 %3, %0
  br i1 %4, label %5, label %9

5:                                                ; preds = %2
  %6 = add i64 %3, 1
  %7 = udiv i64 42, %0
  %8 = icmp slt i64 %3, %7
  br i1 %8, label %2, label %9

9:                                                ; preds = %5, %2
  %10 = phi i64 [ 1, %2 ], [ 0, %5 ]
  ret i64 %10
}

is getting optimized to:

define noundef i64 @f(i64 %0) local_unnamed_addr #0 {
  %smax = tail call i64 @llvm.smax.i64(i64 %0, i64 0)
  %2 = udiv i64 42, %0
  %umin = tail call i64 @llvm.umin.i64(i64 %smax, i64 %2)
  %min.iters.check = icmp ult i64 %umin, 4
  br i1 %min.iters.check, label %scalar.ph.preheader, label %vector.ph

vector.ph:                                        ; preds = %1
  %3 = add nuw nsw i64 %umin, 1
  %n.mod.vf = and i64 %3, 3
  %4 = icmp eq i64 %n.mod.vf, 0
  %5 = select i1 %4, i64 4, i64 %n.mod.vf
  %n.vec = sub nsw i64 %3, %5
  br label %vector.body

vector.body:                                      ; preds = %vector.body, %vector.ph
  %index = phi i64 [ 0, %vector.ph ], [ %index.next, %vector.body ]
  %index.next = add nuw i64 %index, 4
  %6 = icmp eq i64 %index.next, %n.vec
  br i1 %6, label %scalar.ph.preheader, label %vector.body, !llvm.loop !0

scalar.ph.preheader:                              ; preds = %vector.body, %1
  %.ph = phi i64 [ 0, %1 ], [ %n.vec, %vector.body ]
  br label %scalar.ph

scalar.ph:                                        ; preds = %scalar.ph.preheader, %9
  %7 = phi i64 [ %10, %9 ], [ %.ph, %scalar.ph.preheader ]
  %8 = icmp slt i64 %7, %0
  br i1 %8, label %9, label %13

9:                                                ; preds = %scalar.ph
  %10 = add nuw nsw i64 %7, 1
  %11 = udiv i64 42, %0
  %12 = icmp ult i64 %7, %11
  br i1 %12, label %scalar.ph, label %13, !llvm.loop !3

13:                                               ; preds = %9, %scalar.ph
  %14 = phi i64 [ 1, %scalar.ph ], [ 0, %9 ]
  ret i64 %14
}

; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
declare i64 @llvm.smax.i64(i64, i64) #1

; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
declare i64 @llvm.umin.i64(i64, i64) #1

attributes #0 = { nofree norecurse nosync nounwind memory(none) }
attributes #1 = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }

!0 = distinct !{!0, !1, !2}
!1 = !{!"llvm.loop.isvectorized", i32 1}
!2 = !{!"llvm.loop.unroll.runtime.disable"}
!3 = distinct !{!3, !2, !1}

the problem is that the optimized code can divide by zero even when the original code doesn't. to see this, pass 0 as an argument to f. I also independently verified on an x64-64 that the optimized code traps out with an FPE while the original code does not. it's LoopVectorize that's the culprit.

cc @nunoplopes @Hatsunespica
@dtcxzyw
Copy link
Member

dtcxzyw commented Apr 24, 2024

cc @fhahn

@fhahn fhahn self-assigned this Apr 24, 2024
fhahn added a commit that referenced this issue May 14, 2024
@fhahn
[LV] Add tests with trip counts containing UDIV expressions.
cf5db39 
Add test cases for
#89958.
fhahn added a commit to fhahn/llvm-project that referenced this issue May 14, 2024
@fhahn
[LV] Don't vectorize if trip count expansion may introduce UB.
49c699d 
Introduce a utility to check if a SCEV expansion may introduce UB
(couldn't find a similar utility after a quick glance) and use to the
avoid vectorizing when expanding the trip count introduces UB.

Fixes llvm#89958.
@fhahn fhahn linked a pull request May 14, 2024 that will close this issue
Open
mub-at-arm pushed a commit to mub-at-arm/llvm-project that referenced this issue May 16, 2024
@fhahn @mub-at-arm
[LV] Add tests with trip counts containing UDIV expressions.
8af2db8 
Add test cases for
llvm#89958.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@fhahn fhahn

Labels
loopoptim miscompilation vectorization
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[LV] Don't vectorize if trip count expansion may introduce UB. fhahn/llvm-project
3 participants
@fhahn @regehr @dtcxzyw