Missing optimization: loop invariant memset should be hoisted if it's the only memory access in loop

[WanderingAura]

Currently memset is unrolled and optimized within the body of the loop instead of being hoisted, which is suboptimal when the unrolling is less than the iteration number: https://godbolt.org/z/dK3a7xjPW.

This also means -fno-unroll-loops causes the memsets to not be optimized at all.

Previously discussed in #143015:

The optimization happens as a result of unrolling, so it is affected by target-dependent heuristics. It would be legal to do it independently of unrolling by hoisting the memset out of the loop, it's just not implemented. It does work for a plain store, implemented here I believe:

llvm-project/llvm/lib/Transforms/Scalar/LICM.cpp

Line 1269 in 470f456

} else if (auto *SI = dyn_cast<StoreInst>(&I)) {

It could be extended to the memset case.

Originally posted by @nikic in #143015

[WanderingAura]

Hi @EugeneZelenko @nikic, I’d like to try implementing this optimization. Would you consider this a good first issue for someone who’s new to contributing to LLVM? Seems quite small in scope (at least to me) after looking at the source code @nikic mentioned above.

[nikic]

@WanderingAura Yes, this looks pretty simple to me. You'll probably want to handle Behavior.onlyWritesMemory() calls in general rather than memset in particular.

[WanderingAura]

@nikic Thanks, got it. I'll see if I can handle any write only calls that modify memory that isn't being read or written to by other instructions in the loop.

Seems like it's going to be similar to the Behaviour.onlyReadsMemory() logic:
https://github.com/llvm/llvm-project/blob/470f456567fb6c6ee8b6e51c06a64536ca076d74/llvm/lib/Transforms/Scalar/LICM.cpp#L1240C1-L1254C1

It looks like the readonly logic uses pointerInvalidatedByLoop to check the memory isn't being written to elsewhere in the loop. I'm thinking of creating a new function which checks the memory isn't being written to OR read in the relevant sections of the loop.

Do you know if function like that already exists?

[WanderingAura]

@EugeneZelenko Could this issue be assigned to me?