Switch how the datalayout availability test is handled in this code to
authorChandler Carruth <chandlerc@gmail.com>
Sun, 19 Oct 2014 00:42:16 +0000 (00:42 +0000)
committerChandler Carruth <chandlerc@gmail.com>
Sun, 19 Oct 2014 00:42:16 +0000 (00:42 +0000)
commitf84407be32116bd50b9074b51ea3e37ec68515f8
tree885cc2e336a1bc6c449b97cffb9e1ba018675659
parentefed41c6213a84c9edf8d2e74cef2621dfdf3b2e
Switch how the datalayout availability test is handled in this code to
make much more sense and in theory be more correct.

If you trace the code alllll the way back to when it was first
introduced, the comments make it slightly more clear what was going on
here. At that time, the only way Base != V was if DL (then TD) was
non-null. As a consequence, if DL *was* null, that meant we were loading
directly from the alloca or global found above the test. After
refactoring, this has become at least terribly subtle and potentially
incorrect. There are many forms of pointer manipulation that can be
traversed without DataLayout, and some of them would in fact change the
size of object being loaded vs. allocated.

Rather than this subtlety, I've hoisted the actual 'return true' bits
into the code which actually found an alloca or global and based them on
the loaded pointer being that alloca or global. This is both more clear
and safer. I've also added comments about exactly why this set of
predicates is used.

I've also corrected a misleading comment about globals -- if overridden
they may not just have a different size, they may be null and completely
unsafe to load from!

Hopefully this confuses the next reader a bit less. I don't have any
test cases or anything, the patch is motivated strictly to improve the
readability of the code.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@220156 91177308-0d34-0410-b5e6-96231b3b80d8
lib/Analysis/Loads.cpp