LICM introduces load in writeonly function (UB) #51248
Comments
|
This is debatable. Generally attributes like writeonly only refer to caller accessible memory, e.g. it's okay to load from an alloca in a writeonly function. The situation here is similar in that the load is spurious -- the loaded value doesn't actually matter. From the perspective of the caller, while the implementation is not actually writeonly, it behaves as if it were.
The transform is only performed if the store is guaranteed to execute or to an allocation that does not escape before/during the loop. |
If that's the case, then there's no point in doing the load in the first place. The phi can be initialized with poison, as you know you'll never see then 0-iteration value. |
And a global is accessible in caller. Langref says:
So the load could be replaced with unreachable, what means that the code: define void @test(i8 %var) #0 {
entry:
%div = sdiv i8 %var, 3
%cmp2 = icmp slt i8 %div, 0
%glb.promoted = load i8, i8* @glb, align 1could be transformed to: define void @test(i8 %var) #0 {
unreachable |
Test for #51248. LICM introduces an unused load in a writeonly function.
|
Candidate patch: https://reviews.llvm.org/D123473 |
…doesn't exec." This reverts the revert commit ad95255. The updated version also creates a load when the store may not execute. In those cases, we still need to introduce a load in a function where there may not have been one before, so this doesn't completely resolve issue #51248. Original message: When only a store is sunk, there is no need to create a load in the pre-header, as the result of the load will never get used. The dead load can can introduce UB, if the function is marked as writeonly. Reviewed By: nikic Differential Revision: https://reviews.llvm.org/D123473
Extended Description
LICM transforms this:
For functions that are writeonly this introduces UB as the original function had no load and the optimized now has a load from global memory.
The second issue is the store introduction. I didn't check if the store is introduced for loops that are not guaranteed to execute, but if that's the case that may violate C++'s memory model (where I believe you cannot introduce stores).
After LICM:
Reduced test case from John Regehr & Vsevolod Livinskii.
The text was updated successfully, but these errors were encountered: