[PATCH] D158081: [IR] Add writable attribute

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LICM.cpp:2190
+        (!ExplicitlyDereferenceableOnly ||
+         isDereferenceablePointer(SomePtr, AccessTy, MDL)) &&
         isThreadLocalObject(Object, CurLoop, DT, TTI))
----------------
goldstein.w.n wrote:
> nit: Given that the `writeable` attribute is only meaningful in conjunction with `isDeferenceablePointer`, maybe that check should just be part of the `isWritableObject` function? Then you can drop the `ExplicitlyDeferenceableOnly` bool. Just in general seems easier on the user to have the function do the checks of whether the `writable` attribute is meaningful for the object rather than the otherway around.
> 
> In general though I don't see any issues with the code, but am in no way qualified to approve this patch.
> 
My thinking here was that some callers will have already performed the isDereferenceablePointer check anyway, so we can avoid doing it again in isWritableObject. In particular performCallSlotOptzn() already checks isDereferenceableAndAlignedPointer().


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

https://reviews.llvm.org/D158081