Optimizer should consider "maythrow" calls (those without "nounwind) as having side effects. #19286
Comments
|
The test case in this bug report is fixed by I'm closing this because I don't have a test case and I no longer think it makes much sense to mark may-unwind calls "readonly". An unwind path always touches some memory. That fact that it doesn't alias with LLVM-visible memory access can be handled by alias analysis. |
I'm not sure about this -- why does an unwind path have to write some memory? Can't you implement unwind as "read the appropriate exception handler RPC from a table, and return to that RPC"?
Btw, I think in this interpretation it is incorrect for -functionattrs to infer readnone for @foo in (which it does today): define void @foo({ i8*, i32 } %val) personality i8 8 { |
|
Sanjoy, My point was that if a call does write to system memory, even if it doesn't alias with anything exposed to LLVM, then it should not be marked readonly. However, I think the real issue is this... "readonly" is useful to communicate that calls don't need to be sequenced. If it's useful to allow may-unwind calls to be reordered, then I admit it might still be useful to allow readonly on those calls. The -functionattrs behavior looks like a bug to me (how can it assume the memory behavior of the personality function?). But I haven't looked at exception handling recently enough to be aware of the implications one way or the other. At any rate, this PR is stale. Feel free to open new ones that are more to-the-point. |
Extended Description
We should not allow instructions with potential side effects to move
across mayThrow calls. This is an informally accepted interpretation of
the LLVM SPEC. However, in practice, optimizations rely too heavily on
the presence of memory writes to model side effects.
We want to be able to define a call as "readonly" and not "nounwind". We want to be able to eliminate redundant loads across such calls, however, we do not want potentially unsafe loads to be hoisted above such calls. We also do not want to allow such calls to be reordered with respect to each other, and other readonly && mayThrow instructions.
This is important for implementing runtime calls, as commonly needed for high level language constructs. It also allows llvm.experimental.stackmap to be modeled correctly.
The purpose of this PR is to complete an audit of the optimizer/codegen to catch cases that violate the assumptions above, improve the API, and better document this requirement.
Here is just one example where codegen violates the rule:
int32_t foo(int32_t* ptr) {
int i = 0;
int result;
do {
bar(ptr);
result = *ptr;
bar(ptr);
} while (i++ < *ptr);
return result;
}
declare void @bar(i32*) readonly;
(So 'bar' is 'readonly' and 'may-unwind')
When LICM tries to sink the calls to 'bar', it sees the 'readonly', assumes no side effects and sinks it below the loads.
The text was updated successfully, but these errors were encountered: