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.

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

After r313012, Assertion failed: (ExitCount != SE.getCouldNotCompute() && "Invalid loop count"), function generateOverflowCheck, file lib/Analysis/ScalarEvolutionExpander.cpp, line 2126. #35380

Closed
DimitryAndric opened this issue Jan 21, 2018 · 13 comments
Labels
bugzilla Issues migrated from bugzilla

Comments

@DimitryAndric
Copy link
Collaborator

Bugzilla Link 36032
Resolution FIXED
Resolved on Feb 26, 2018 16:26
Version trunk
OS All
Blocks #35152
CC @aelovikov-intel,@emaste,@zmodem,@sbaranga-arm

Extended Description

As reported in https://bugs.freebsd.org/225345, building the FreeBSD games/scummvm port for AArch64 with clang 6.0.0 results in an assertion:

Assertion failed: (ExitCount != SE.getCouldNotCompute() && "Invalid loop count"), function generateOverflowCheck, file /usr/local/poudriere/jails/head-arm64/usr/src/contrib/llvm/lib/Analysis/ScalarEvolutionExpander.cpp, line 2152.

This assertion still occurs with trunk r322755, and bisection shows it has regressed with https://reviews.llvm.org/rL313012 ("[LAA] Allow more run-time alias checks by coercing pointer expressions to AddRecExprs") by Silviu Baranga.

Minimized test case:

// clang -cc1 -triple aarch64-- -S -O1 -vectorize-loops graphics-minimized.cpp
struct {
void *a();
} b;
char c[6];
void d() {
unsigned char *e = (unsigned char *)b.a();
unsigned short f;
for (;;) {
for (unsigned g; g < 4; g++)
e[g] = c[g + f];
f += 4;
}
}

Note that -vectorize-loops is essential for reproducing the assertion.

@sbaranga-arm
Copy link
Mannequin

sbaranga-arm mannequin commented Jan 22, 2018

This uncovers an issue introduced in r308299.

createAddRecFromPHIWithCasts shouldn't create predicates for the loop that isn't currently processed by the SCEV rewriter because we don't have a guarantee that we have the backedge taken count.

@​Dorit: could you have a look please?

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 26, 2018

The patch should fix the issue: https://reviews.llvm.org/D42604

@zmodem
Copy link
Collaborator

zmodem commented Feb 19, 2018

The patch should fix the issue: https://reviews.llvm.org/D42604

Evgeny, it looks like the patch is stuck?

@zmodem
Copy link
Collaborator

zmodem commented Feb 21, 2018

The patch should fix the issue: https://reviews.llvm.org/D42604

Evgeny, it looks like the patch is stuck?

Evgeny, Silviu: it looks like the patch is ready to be committed? This needs to land soon to be able to go into the release.

@sbaranga-arm
Copy link
Mannequin

sbaranga-arm mannequin commented Feb 21, 2018

Yes, I think the patch should be ready to commit, but Evgeny seems unresponsive at the moment.

@sbaranga-arm
Copy link
Mannequin

sbaranga-arm mannequin commented Feb 21, 2018

Ok, I'll turn off versioning for this case so that we can get the fix in time.

@sbaranga-arm
Copy link
Mannequin

sbaranga-arm mannequin commented Feb 21, 2018

I've disabled the buggy feature in r325687 until we can get Evgeny's fix in.

@zmodem
Copy link
Collaborator

zmodem commented Feb 22, 2018

I've disabled the buggy feature in r325687 until we can get Evgeny's fix in.

Should we merge that, or what's the story for 6.0.0?

@sbaranga-arm
Copy link
Mannequin

sbaranga-arm mannequin commented Feb 22, 2018

Yes, let's merge r325687 for 6.0.0.

@zmodem
Copy link
Collaborator

zmodem commented Feb 22, 2018

Yes, let's merge r325687 for 6.0.0.

Merged in r325773. Thanks!

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 22, 2018

I've disabled the buggy feature in r325687 until we can get Evgeny's fix in.

@​Silviu,

Evgeny is out of the office this week. He should be returning on Feb-26. I'll ask him to commit his fix when he returns.

Thanks,
-Eric

@sbaranga-arm
Copy link
Mannequin

sbaranga-arm mannequin commented Feb 23, 2018

Sure, no problem. r325687 should also be reverted once the fix goes in.

-Silviu

I've disabled the buggy feature in r325687 until we can get Evgeny's fix in.

@​Silviu,

Evgeny is out of the office this week. He should be returning on Feb-26.
I'll ask him to commit his fix when he returns.

Thanks,
-Eric

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 27, 2018

Committed to trunk revision 326154.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla
Projects
None yet
Development

No branches or pull requests

3 participants