[PATCH] D106947: [DSE][NFC] Rename Later->Killing, Earlier->Killed

Evgeniy via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 7 20:19:05 PDT 2021


ebrevnov added a comment.



> I think there are some places where the killing/dead terminology is a bit inaccurate for intermediate steps, e.g the first changed comment ` // 1. Get the next dominating clobbering MemoryDef (KilledAccess) by walking`. At this point, it is not yet known whether the access is dead/killed. It is just a subtle difference in wording, but I think it might be worth to try to encode it in the name. Using `MaybeDead/PotentiallyDead` as  suggested by @asbirlea  in such cases seem good options. IMO the `Maybe` prefix is easier to follow and more compact and personally I am not too worried about it also being used for `Optional`.

Please take a look now.



================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:340
+/// overwrite one another. More specifically, \p KillingI has to
+/// overwrite \p DeadII.
+static OverwriteResult isMaskedStoreOverwrite(const Instruction *KillingI,
----------------
asbirlea wrote:
> s/DeadII/DeadI
Fixed. Thanks for noting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106947



More information about the llvm-commits mailing list