llvm dev - Jan 2019 - stale info in the assumption cache

Hi all,

We have recently encountered a problem with AssumptionCache that it does not get
updated when a block with llvm.assume calls gets outlined by the CodeExtractor.
As a result we end up with stale references to the llvm.assume calls that were
moved to the outlined function in the parent function's cache.

The problem can be reproduced on the attached file as follows (many thanks to
Andy Kaylor for creating this reproducer)

$ opt -slp-vectorizer -hotcoldsplit -slp-vectorizer -S -o - extract.ll
opt: .../llvm/lib/Analysis/CodeMetrics.cpp:107: static void
llvm::CodeMetrics::collectEphemeralValues(const llvm::Function*,
llvm::AssumptionCache*, llvm::SmallPtrSetImpl<const llvm::Value*>&):
Assertion `I->getParent()->getParent() == F && "Found
assumption for the wrong function!"' failed.
...

The comments in the AssumptionCache seem to indicate that we should never get
into this situation because AssumptionCache is supposed to be self-updating, but
the case when a block with llvm.assume call is moved from one function to
another is not correctly handled now. I wonder what the right way to fix this
problem is.

I guess a straightforward solution would be just changing CodeExtractor to clear
function's assumption cache right after extracting blocks from it, but that
would require adding assumption analysis as a dependency to every pass that uses
CodeExtractor.

Or maybe there could be other solutions like for example somehow extending
callback mechanism to handle updates of the basic block parent. Can you please
suggest any ideas?

Thanks,
Sergey
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190117/49034c3b/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: extract.ll
Type: application/octet-stream
Size: 515 bytes
Desc: extract.ll
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190117/49034c3b/attachment.obj>

On 1/16/19 6:59 PM, Dmitriev, Serguei N via llvm-dev wrote:
Hi all,

We have recently encountered a problem with AssumptionCache that it does not get
updated when a block with llvm.assume calls gets outlined by the CodeExtractor.
As a result we end up with stale references to the llvm.assume calls that were
moved to the outlined function in the parent function’s cache.

The problem can be reproduced on the attached file as follows (many thanks to
Andy Kaylor for creating this reproducer)

$ opt -slp-vectorizer -hotcoldsplit -slp-vectorizer -S -o - extract.ll
opt: .../llvm/lib/Analysis/CodeMetrics.cpp:107: static void
llvm::CodeMetrics::collectEphemeralValues(const llvm::Function*,
llvm::AssumptionCache*, llvm::SmallPtrSetImpl<const llvm::Value*>&):
Assertion `I->getParent()->getParent() == F && "Found
assumption for the wrong function!"' failed.
...

The comments in the AssumptionCache seem to indicate that we should never get
into this situation because AssumptionCache is supposed to be self-updating, but
the case when a block with llvm.assume call is moved from one function to
another is not correctly handled now. I wonder what the right way to fix this
problem is.

I guess a straightforward solution would be just changing CodeExtractor to clear
function’s assumption cache right after extracting blocks from it, but that
would require adding assumption analysis as a dependency to every pass that uses
CodeExtractor.


I don't, on first impression, have a better idea.


Or maybe there could be other solutions like for example somehow extending
callback mechanism to handle updates of the basic block parent. Can you please
suggest any ideas?


I don't think that our current ValueHandle callbacks will give us this
functionality. We'd need a new callback?


Thanks again,

Hal


Thanks,
Sergey



_______________________________________________
LLVM Developers mailing list
llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev


--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190117/0827f64d/attachment.html>

Hi Hal,

On 1/16/19 6:59 PM, Dmitriev, Serguei N via llvm-dev wrote:
Hi all,

We have recently encountered a problem with AssumptionCache that it does not get
updated when a block with llvm.assume calls gets outlined by the CodeExtractor.
As a result we end up with stale references to the llvm.assume calls that were
moved to the outlined function in the parent function's cache.

The problem can be reproduced on the attached file as follows (many thanks to
Andy Kaylor for creating this reproducer)

$ opt -slp-vectorizer -hotcoldsplit -slp-vectorizer -S -o - extract.ll
opt: .../llvm/lib/Analysis/CodeMetrics.cpp:107: static void
llvm::CodeMetrics::collectEphemeralValues(const llvm::Function*,
llvm::AssumptionCache*, llvm::SmallPtrSetImpl<const llvm::Value*>&):
Assertion `I->getParent()->getParent() == F && "Found
assumption for the wrong function!"' failed.
...

The comments in the AssumptionCache seem to indicate that we should never get
into this situation because AssumptionCache is supposed to be self-updating, but
the case when a block with llvm.assume call is moved from one function to
another is not correctly handled now. I wonder what the right way to fix this
problem is.

I guess a straightforward solution would be just changing CodeExtractor to clear
function's assumption cache right after extracting blocks from it, but that
would require adding assumption analysis as a dependency to every pass that uses
CodeExtractor.



I don't, on first impression, have a better idea.


Ok, I will probably implement this solution so far.

Or maybe there could be other solutions like for example somehow extending
callback mechanism to handle updates of the basic block parent. Can you please
suggest any ideas?



I don't think that our current ValueHandle callbacks will give us this
functionality. We'd need a new callback?


I also think that existing callbacks cannot handle the case when block's
parent is changed, so something new will be needed. AssumptionCache as I
understand was designed to be self-updating, so maybe it would be nice to
maintain this property.

Thanks,
Sergey


Thanks again,

Hal



Thanks,
Sergey



_______________________________________________

LLVM Developers mailing list

llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>

http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

--

Hal Finkel

Lead, Compiler Technology and Programming Languages

Leadership Computing Facility

Argonne National Laboratory
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190117/15aaccdd/attachment.html>