[PATCH] D64258: [InferFuncAttributes] extend 'dereferenceable' attribute based on loads

Hal Finkel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 8 15:09:23 PDT 2019


hfinkel added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/InferFunctionAttrs.cpp:40
+    Value *PtrOp;
+    if (!match(&I, m_Load(m_Value(PtrOp))))
+      continue;
----------------
Why not also stores (or AtomicCmpXchg or AtomicRMW)?


================
Comment at: llvm/lib/Transforms/IPO/InferFunctionAttrs.cpp:44
+    // The implicit GEP index for that pattern is zero.
+    auto *Arg = dyn_cast<Argument>(PtrOp);
+    uint64_t GepIndex = 0;
----------------
This logic seems unnecessarily limited. Why not use GetPointerBaseWithConstantOffset?


================
Comment at: llvm/lib/Transforms/IPO/InferFunctionAttrs.cpp:87
+    for (uint64_t Index : GepIndexes) {
+      if (Index != MaxGepIndex)
+        break;
----------------
Does this do the right thing if the lowest GEP index is negative? You could skip the negative ones first?

As a general point, I think that you want logic here mirroring (a subset of) what's in isOverwrite in DeadStoreElimination.cpp


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

https://reviews.llvm.org/D64258





More information about the llvm-commits mailing list