[PATCH] D66778: [Loads/SROA] Remove blatantly incorrect code and fix a bug revealed in the process

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 27 12:08:52 PDT 2019


jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

Removing the duplicated (wrong) logic is still fine. Using `Size` is also the right thing. The bits/bytes size mixup seems unrelated but given that bytes is the right thing I'm OK with it.

The one thing I would like to add is a test for the `Size` vs `PtrSize` issue, see my comment below. Otherwise, LGTM.



================
Comment at: lib/Analysis/Loads.cpp:268
+    if (AccessedPtr == V &&
+        LoadSize <= DL.getTypeStoreSize(AccessedTy))
       return true;
----------------
Can we have a test for this? I would hope something where we want to pre-loads a 64 bit pointer but only have a 32 bit access?


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

https://reviews.llvm.org/D66778





More information about the llvm-commits mailing list