[PATCH] D102959: [polly] Fix SCEVLoopAddRecRewriter to avoid invalid AddRecs.

2 views
Skip to first unread message

Eli Friedman via Phabricator

unread,
May 21, 2021, 8:12:54 PMMay 21
to efri...@quicinc.com, ll...@meinersbur.de, siddu...@gmail.com, llvm-c...@lists.llvm.org, poll...@googlegroups.com, bhuvanend...@amd.com, 888...@gmail.com, doug...@gmail.com, w...@google.com, czhe...@cn.ibm.com, ruilin...@amd.com, kuh...@google.com
efriedma created this revision.
efriedma added a reviewer: Meinersbur.
Herald added a reviewer: bollu.
Herald added a subscriber: hiraditya.
efriedma requested review of this revision.
Herald added a project: LLVM.

When we're remapping an AddRec, the AddRec constructed by a partial rewrite might not make sense. This triggers an assertion complaining it's not loop-invariant.

Instead of constructing the partially rewritten AddRec, just skip straight to calling evaluateAtIteration.

Testcase was automatically reduced using llvm-reduce, so it's a little messy, but hopefully makes sense.


Repository:
rG LLVM Github Monorepo

https://reviews.llvm.org/D102959

Files:
llvm/include/llvm/Analysis/ScalarEvolutionExpressions.h
llvm/lib/Analysis/ScalarEvolution.cpp
polly/test/Isl/CodeGen/OpenMP/scev-rewriting.ll

D102959.347155.patch

Michael Kruse via Phabricator

unread,
May 25, 2021, 3:06:24 PMMay 25
to efri...@quicinc.com, siddu...@gmail.com, llvm-c...@lists.llvm.org, poll...@googlegroups.com, bhuvanend...@amd.com, 888...@gmail.com, doug...@gmail.com, w...@google.com, czhe...@cn.ibm.com, ruilin...@amd.com, kuh...@google.com
Meinersbur added a comment.

This only changes code in ScalarEvolution, so is this a SCEV fix or was Polly using ScalarEvolution in a way that was not originally intended?


Repository:
rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D102959/new/

https://reviews.llvm.org/D102959

Eli Friedman via Phabricator

unread,
May 27, 2021, 5:48:25 PMMay 27
to efri...@quicinc.com, ll...@meinersbur.de, siddu...@gmail.com, llvm-c...@lists.llvm.org, poll...@googlegroups.com, bhuvanend...@amd.com, 888...@gmail.com, doug...@gmail.com, w...@google.com, czhe...@cn.ibm.com, ruilin...@amd.com, kuh...@google.com
efriedma added a comment.

SCEVLoopAddRecRewriter was originally added for use by polly, and has only ever been used by polly. So the "original intent" is just whatever polly needed, I think.

Michael Kruse via Phabricator

unread,
May 27, 2021, 9:09:04 PMMay 27
to efri...@quicinc.com, siddu...@gmail.com, llvm-c...@lists.llvm.org, poll...@googlegroups.com, bhuvanend...@amd.com, 888...@gmail.com, doug...@gmail.com, w...@google.com, czhe...@cn.ibm.com, ruilin...@amd.com, kuh...@google.com
Meinersbur accepted this revision.
Meinersbur added a comment.
This revision is now accepted and ready to land.

The test is sufficient for me. Thank you for the fix.

Eli Friedman via Phabricator

unread,
Jun 1, 2021, 12:51:32 PMJun 1
to efri...@quicinc.com, ll...@meinersbur.de, siddu...@gmail.com, llvm-c...@lists.llvm.org, poll...@googlegroups.com, bhuvanend...@amd.com, 888...@gmail.com, doug...@gmail.com, w...@google.com, czhe...@cn.ibm.com, ruilin...@amd.com, kuh...@google.com
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfd229caa0138: [polly] Fix SCEVLoopAddRecRewriter to avoid invalid AddRecs. (authored by efriedma).

Repository:
rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D102959/new/

https://reviews.llvm.org/D102959

Files:
llvm/include/llvm/Analysis/ScalarEvolutionExpressions.h
llvm/lib/Analysis/ScalarEvolution.cpp
polly/test/Isl/CodeGen/OpenMP/scev-rewriting.ll

D102959.348996.patch
Reply all
Reply to author
Forward
0 new messages