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

Incorrect MachineSink into clobbering loop #53990

Closed
nikic opened this issue Feb 22, 2022 · 5 comments
Closed

Incorrect MachineSink into clobbering loop #53990

nikic opened this issue Feb 22, 2022 · 5 comments
Labels
llvm:codegen release:backport release:merged release:reviewed
Milestone
LLVM 14.0.0 Release

Comments

@nikic
Copy link
Contributor

nikic commented Feb 22, 2022 

; RUN: llc -mtriple=x86_64-- < %s
declare void @clobber()

define void @test(i1 %c, i64* %p, i64* noalias %p2) nounwind {
entry:
  %val = load i64, i64* %p, align 8
  br label %loop

loop:
  switch i8 undef, label %unreachable [
    i8 0, label %latch
    i8 1, label %split.1
    i8 2, label %split.2
    i8 3, label %split.3
  ]

unreachable:
  unreachable

split.3:
  br i1 %c, label %clobber, label %sink

split.1:
  br label %latch

split.2:
  br label %latch

clobber:
  call void @clobber()
  br label %sink

sink:
  store i64 %val, i64* %p2, align 8
  br label %latch

latch:
  %phi = phi i64 [ 0, %sink ], [ 0, %split.2 ], [ 1, %split.1 ], [ 0, %loop ]
  %phi.live = add i64 %phi, 0
  br label %loop
}

The load is sunk into split.3, but may then be clobbered by following the split.3 -> clobber -> sink -> latch -> loop -> split.3 chain.

This is due to the functionality introduced in https://reviews.llvm.org/D86864, which uses incorrect post-dominance reasoning.
@nikic nikic mentioned this issue Feb 22, 2022
Closed
@nikic nikic added this to the LLVM 14.0.0 Release milestone Feb 22, 2022
@nikic
Copy link
Contributor Author

nikic commented Feb 22, 2022

Candidate patch: https://reviews.llvm.org/D120330

@nikic nikic closed this as completed in 6fde043 Mar 2, 2022
@nikic nikic reopened this Mar 3, 2022
@nikic
Copy link
Contributor Author

nikic commented Mar 3, 2022

/cherry-pick e075bf6 6fde043

@llvmbot
Copy link
Member

llvmbot commented Mar 3, 2022

/branch llvmbot/llvm-project/issue53990

llvmbot pushed a commit to llvmbot/llvm-project that referenced this issue Mar 3, 2022
@nikic @llvmbot
[MachineSink] Disable if there are any irreducible cycles
ce074ad 
This is an alternative to D120330, which disables MachineSink for
functions with irreducible cycles entirely. This avoids both the
correctness problem, and ensures we don't perform non-profitable
sinks into cycles. At the same time, it may also disable
profitable sinks in the same function. This can be made more
precise by using MachineCycleInfo in the future.

Fixes llvm#53990.

Differential Revision: https://reviews.llvm.org/D120800

(cherry picked from commit 6fde043)
@llvmbot llvmbot mentioned this issue Mar 3, 2022
Merged
@llvmbot
Copy link
Member

llvmbot commented Mar 3, 2022

/pull-request llvmbot#114

tstellar pushed a commit that referenced this issue Mar 8, 2022
@nikic @tstellar
[MachineSink] Disable if there are any irreducible cycles
6755510 
This is an alternative to D120330, which disables MachineSink for
functions with irreducible cycles entirely. This avoids both the
correctness problem, and ensures we don't perform non-profitable
sinks into cycles. At the same time, it may also disable
profitable sinks in the same function. This can be made more
precise by using MachineCycleInfo in the future.

Fixes #53990.

Differential Revision: https://reviews.llvm.org/D120800

(cherry picked from commit 6fde043)
@tstellar
Copy link
Collaborator

tstellar commented Mar 8, 2022 •
edited
Loading

Merged: 1e4fd59 6755510

@tstellar tstellar closed this as completed Mar 8, 2022
mem-frob pushed a commit to draperlaboratory/hope-llvm-project that referenced this issue Oct 7, 2022
@nikic
[MachineSink] Disable if there are any irreducible cycles
6ed60a5 
This is an alternative to D120330, which disables MachineSink for
functions with irreducible cycles entirely. This avoids both the
correctness problem, and ensures we don't perform non-profitable
sinks into cycles. At the same time, it may also disable
profitable sinks in the same function. This can be made more
precise by using MachineCycleInfo in the future.

Fixes llvm/llvm-project#53990.

Differential Revision: https://reviews.llvm.org/D120800
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
llvm:codegen release:backport release:merged release:reviewed
Projects
None yet
Milestone
LLVM 14.0.0 Release
Development

No branches or pull requests
4 participants
@tstellar @nikic @EugeneZelenko @llvmbot