[PATCH] D124967: [SROA] Avoid postponing rewriting load/store by ignoring lifetime intrinsics in partition's promotability checking

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

I've done some due diligence, and the relevant difference starts with this code: https://github.com/llvm/llvm-project/blob/1ddc6ab1a9c321e02da4fab836c5f863a906108b/llvm/lib/Transforms/Scalar/SROA.cpp#L4412 If lifetime intrinsics are present, we'll mark both the `[0,2)` and `[0,4)` slices as unsplittable, and will then visit the `[0,4)` partition first, which has viable integer widening. Without lifetime intrinsics, we'll mark only `[0,2)` as unsplittable, and visit partition `[0,2)` first, where widening is not viable. I think something is very wrong here, but I'm not confident in which place it is and don't want to spend a lot of time on this.

In any case, I think your patch makes sense on a conceptual level, so let's go with it for now.



================
Comment at: llvm/lib/Transforms/Scalar/SROA.cpp:1993
+      return true;
+  }
+
----------------
These already is a check for `isLifetimeStartOrEnd()` and `isDroppable()` below, only difference is that it's after the `RelEnd > Size` check. I think you'd want to add the `isDroppable()` check here as well, and then drop the IntrinsicInst handling below entirely.

You can also move the `Use *U = S.getUse();` line before here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124967/new/

https://reviews.llvm.org/D124967