[PATCH] D114035: [DSE] Improve handling of `strncpy` in Dead Store Elimination

Thu Nov 18 08:21:27 PST 2021

fhahn added a comment.

In D114035#3139988 <https://reviews.llvm.org/D114035#3139988>, @xbolva00 wrote:

> In D114035#3139813 <https://reviews.llvm.org/D114035#3139813>, @fhahn wrote:
>
>> Is there a way to add the right attributes to `strncpy` to avoid needing to special case it in DSE?
>
> It would be great to somehow connect write_only/read_only with size arg but I dont think this is possible now. GCC’s access attribute is more poweful and it can be used in this way.

Yeah, but I was actually thinking about the special handling for `isReadClobber` (see inline comments). I don't think the special handling there is needed, because should `strncpy` already have the required attributes to determine it is not a read clobber.

================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1441
+          isReadClobber(KillingLoc, CurrentI)) {
+        if (const auto *CB = dyn_cast<CallBase>(CurrentI)) {
+          LibFunc LF;
----------------
fhahn wrote:
> having this logic here makes the code harder to read. Can this be moved to `isReadClobber`? Or into `BasicAAResult::getModRefInfo` directly?
Actually this should not be needed, because `strncpy` already *has* the correct attributes (first arg write only, second arg readonly and function argmemonly), right?

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

https://reviews.llvm.org/D114035