It was inaccessiblemem: readwrite before, no need for the read.
No real benefit is expected but it can help debugging and other efforts.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
For some reason, the vectorizer is impacted by write only on the assume:
https://godbolt.org/z/MM5W9rE3M
I doubt this is good.
The problem is that we only check intrinsics with read effects, so this code won't be executed with this patch (except if we hoist it out of the conditional):
// Many math library functions read the rounding mode. We will only // vectorize a loop if it contains known function calls that don't set // the flag. Therefore, it is safe to ignore this read from memory. auto *Call = dyn_cast<CallInst>(&I); if (Call && getVectorIntrinsicIDForCall(Call, TLI)) continue;
Assuming that assume can only be used for path-/flow- insensitive invariants (that hold for the whole program), this change is correct. The implication is that the order of assumes doesn't matter.
Alas, with the previous spec, it was already possible to hoist assumes out of branches, so it implied it couldn't be used to assume facts just in a specific branch.
I need to change Alive2's semantics to make sure that LLVM is using assume in this way only.
Assume definitely needs to be flow sensitive. But I don't understand why this change would remove the flow sensitivity. Assume is still non-speculatable, so it cannot be moved outside of branches.
Ah, never mind, I don't know what I was thinking when I wrote that comment 😅 I forgot for a second that assume is a function call in LLVM IR.
Sorry, it's all good.