Some questions about InstCombineLoadStoreAlloca.cpp

IR & Optimizations
#1

We’ve noticed the function “isOnlyCopiedFromConstantGlobal” showing up in VTune on an example with slow compilation time (inside a JIT) and I was trying to see if I could simplify it. Although, I suspect the real problem is that it is being called quadratically from InstCombine - we’re looking in that as well. More on that later, hopefully.

In order to understand the code more easily I restructured it to use the InstVisitor instead of a mix of early returns and continues. This version passes all of the unit tests. I left the old code there in comments to try to make it easier to reason about if the two implementations should have the same behavior.

To me, the old code was hard to reason about due to the interleaved “return false” and “continue” - this version tries to tame that control flow a bit. One thing I was not super confident about is “I.isLifetimeStartOrEnd” - is my reasoning correct that you only really need to check that on Call instructions?

Thanks for any thoughts on the topic.

#2

Lifetime start/end are intrinsics, so yes, it’s sufficient to check it on Call instructions.

Regarding the compile-time issue: These kinds of ad-hoc use walks (especially if they are recursive!) generally need to be limited. See for example llvm-project/InstructionCombining.cpp at 9cf17ac04a7604b59ccfe27ba0b7e335f54a3a76 · llvm/llvm-project · GitHub for a similar limit in InstCombine.

This is particularly problematic in InstCombine, because it may revisit instructions many times – for example, if a user gets removed, the instruction will be revisited and perform the walk again.

1 Like