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

"Instruction does not dominate all uses" with opt -disable-basicaa -loop-versioning #36998

Open
mikaelholmen opened this issue Jun 1, 2018 · 4 comments · May be fixed by #116443
Open

"Instruction does not dominate all uses" with opt -disable-basicaa -loop-versioning #36998

mikaelholmen opened this issue Jun 1, 2018 · 4 comments · May be fixed by #116443
Labels
bugzilla Issues migrated from bugzilla llvm:crash llvm:optimizations

Comments

@mikaelholmen
Copy link
Collaborator

Bugzilla Link 37650
Version unspecified
OS Linux
Attachments reproducer

Extended Description

opt -S -o - bbi-14805.ll -disable-basicaa -loop-versioning

gives

Instruction does not dominate all uses!
%inc = add nsw i16 undef, 1
store i16 %inc, i16* @​c, align 1
LLVM ERROR: Broken function found, compilation aborted!

Found when running random passes/flags on random input. We don't normally use -disable-basicaa -loop-versioning.

This happens also with some old opt binary built in Nov 2016.
@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
@fhahn
Copy link
Contributor

fhahn commented Nov 22, 2022

Still reproduces with -passes=loop-versioning -aa-pipeline='': https://clang.godbolt.org/z/xMKo5P989

@VedantParanjape
Copy link

VedantParanjape commented Nov 15, 2024 •
edited
Loading 

target triple = "x86_64-unknown-linux-gnu"

@a = external global i16, align 1
@b = external global i16, align 1
@c = external global i16, align 1

define void @f2() {
entry:
  br label %for.body

for.body:                                         ; preds = %for.body, %entry
  %0 = load i16, ptr @a, align 1
  store i16 %0, ptr @b, align 1
  %inc = add nsw i16 undef, 1
  br i1 false, label %for.body, label %for.cond.for.end_crit_edge

for.cond.for.end_crit_edge:                       ; preds = %for.body
  %split2 = phi i16 [ %inc, %for.body ]
  store i16 %inc, ptr @c, align 1
  ret void
}

Converted the testcase to opaque pointers. Able to reproduce with opt -passes=loop-versioning -aa-pipeline='' crash.ll

I want to work on this issue, can I assign this to self?

@VedantParanjape
Copy link

Still reproduces with -passes=loop-versioning -aa-pipeline='': https://clang.godbolt.org/z/xMKo5P989

The IR ins't in LCSSA form, in the critical edge, store uses inc which is a value defined inside the loop.

VedantParanjape added a commit to VedantParanjape/llvm-project that referenced this issue Nov 15, 2024
@VedantParanjape
[LoopVersioning] Add a check to see if the input loop is in LCSSA form
28ff01f 
Loop Optimizations expect the input loop to be in LCSSA form. But it seems
that LoopVersioning doesn't have any check to see if the loop is actually in
LCSSA form. As a result, if we give it a loop which is not in LCSSA form but
still correct semantically, the resulting transformation fails to pass through
verifier pass with the following error.

Instruction does not dominate all uses!
%inc = add nsw i16 undef, 1
store i16 %inc, ptr @c, align 1

As the loop is not in LCSSA form, LoopVersioning's transformations leads to
invalid IR! As some instructions do not dominate all their uses.

This patch checks if a loop is in LCSSA form, if not it will call
formLCSSARecursively on the loop before passing it to LoopVersioning.

Fixes: llvm#36998
@VedantParanjape VedantParanjape linked a pull request Nov 15, 2024 that will close this issue
Open
VedantParanjape added a commit to VedantParanjape/llvm-project that referenced this issue Nov 15, 2024
@VedantParanjape
[LoopVersioning] Add a check to see if the input loop is in LCSSA form
bfc6152 
Loop Optimizations expect the input loop to be in LCSSA form. But it seems
that LoopVersioning doesn't have any check to see if the loop is actually in
LCSSA form. As a result, if we give it a loop which is not in LCSSA form but
still correct semantically, the resulting transformation fails to pass through
verifier pass with the following error.

Instruction does not dominate all uses!
%inc = add nsw i16 undef, 1
store i16 %inc, ptr @c, align 1

As the loop is not in LCSSA form, LoopVersioning's transformations leads to
invalid IR! As some instructions do not dominate all their uses.

This patch checks if a loop is in LCSSA form, if not it will call
formLCSSARecursively on the loop before passing it to LoopVersioning.

Fixes: llvm#36998
@VedantParanjape
Copy link

I have pushed a fix for this, thanks (cc: @fhahn and @aeubanks) (#116443)

Thanks,
Vedant

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bugzilla Issues migrated from bugzilla llvm:crash llvm:optimizations
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[LoopVersioning] Add a check to see if the input loop is in LCSSA form VedantParanjape/llvm-project
4 participants
@fhahn @EugeneZelenko @VedantParanjape @mikaelholmen