Commit Graph

22 Commits

Author SHA1 Message Date
Chandler Carruth
b7ea0ce700 Fix a long-standing miscompile in the load analysis that was uncovered
by my refactoring of this code.

The method isSafeToLoadUnconditionally assumes that the load will
proceed with the preferred type alignment. Given that, it has to ensure
that the alloca or global is at least that aligned. It has always done
this historically when a datalayout is present, but has never checked it
when the datalayout is absent. When I refactored the code in r220156,
I exposed this path when datalayout was present and that turned the
latent bug into a patent bug.

This fixes the issue by just removing the special case which allows
folding things without datalayout. This isn't worth the complexity of
trying to tease apart when it is or isn't safe without actually knowing
the preferred alignment.

llvm-svn: 220161
2014-10-19 08:17:50 +00:00
Chandler Carruth
318612525a 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.

llvm-svn: 220156
2014-10-19 00:42:16 +00:00
Chandler Carruth
4768aadcc2 Rename 'TD' to 'DL' in this function as the argument is now a DataLayout
argument.

llvm-svn: 220151
2014-10-18 23:47:22 +00:00
Chandler Carruth
480012ecff Fix the other comment to use modern doxygen style and be a bit more
direct. Notably, comment on the fact that the loaded type is significant
in that it determines how wide of an access must be safe.

llvm-svn: 220150
2014-10-18 23:46:17 +00:00
Chandler Carruth
5cac61e244 More formatting cleanup brought to you by clang-format.
llvm-svn: 220149
2014-10-18 23:41:25 +00:00
Chandler Carruth
693361f496 Clean up doxygen syntax and reword comments to flow better, have a brief
section, and not have unfinished sentence fragments.

llvm-svn: 220147
2014-10-18 23:31:55 +00:00
Chandler Carruth
908f1ee18d Clean up the formatting and trailing whitespace of a routine before
editting it.

llvm-svn: 220146
2014-10-18 23:19:03 +00:00
Hal Finkel
9be4aefa57 AA metadata refactoring (introduce AAMDNodes)
In order to enable the preservation of noalias function parameter information
after inlining, and the representation of block-level __restrict__ pointer
information (etc.), additional kinds of aliasing metadata will be introduced.
This metadata needs to be carried around in AliasAnalysis::Location objects
(and MMOs at the SDAG level), and so we need to generalize the current scheme
(which is hard-coded to just one TBAA MDNode*).

This commit introduces only the necessary refactoring to allow for the
introduction of other aliasing metadata types, but does not actually introduce
any (that will come in a follow-up commit). What it does introduce is a new
AAMDNodes structure to hold all of the aliasing metadata nodes associated with
a particular memory-accessing instruction, and uses that structure instead of
the raw MDNode* in AliasAnalysis::Location, etc.

No functionality change intended.

llvm-svn: 213859
2014-07-24 12:16:19 +00:00
Craig Topper
8cd194d4c1 [C++11] More 'nullptr' conversion. In some cases just using a boolean check instead of comparing to nullptr.
llvm-svn: 206243
2014-04-15 04:59:12 +00:00
Dan Gohman
7eac0c2694 Change GetPointerBaseWithConstantOffset's DataLayout argument from a
reference to a pointer, so that it can handle the case where DataLayout
is not available and behave conservatively.

llvm-svn: 174024
2013-01-31 02:00:45 +00:00
Chandler Carruth
4c1f3c24db Move all of the header files which are involved in modelling the LLVM IR
into their new header subdirectory: include/llvm/IR. This matches the
directory structure of lib, and begins to correct a long standing point
of file layout clutter in LLVM.

There are still more header files to move here, but I wanted to handle
them in separate commits to make tracking what files make sense at each
layer easier.

The only really questionable files here are the target intrinsic
tablegen files. But that's a battle I'd rather not fight today.

I've updated both CMake and Makefile build systems (I think, and my
tests think, but I may have missed something).

I've also re-sorted the includes throughout the project. I'll be
committing updates to Clang, DragonEgg, and Polly momentarily.

llvm-svn: 171366
2013-01-02 11:36:10 +00:00
Nuno Lopes
b0daac8fad use ValueTracking's GetPointerBaseWithConstantOffset() function instead of a local implementation
llvm-svn: 171307
2012-12-31 17:42:11 +00:00
Micah Villmow
bb1a25cd67 Move TargetData to DataLayout.
llvm-svn: 165402
2012-10-08 16:38:25 +00:00
Chris Lattner
84f83c2727 enhance jump threading to preserve TBAA information when PRE'ing loads,
fixing rdar://11039258, an issue that came up when inspecting clang's 
bootstrapped codegen.

llvm-svn: 152635
2012-03-13 18:07:41 +00:00
Eli Friedman
a88908f4e7 Add some comments here because the lack of a check for volatile/atomic here is a bit unusual.
llvm-svn: 137662
2011-08-15 21:56:39 +00:00
Jay Foad
0974b71f17 Convert TargetData::getIndexedOffset to use ArrayRef.
llvm-svn: 135478
2011-07-19 14:01:37 +00:00
Chris Lattner
e1fe7061ce land David Blaikie's patch to de-constify Type, with a few tweaks.
llvm-svn: 135375
2011-07-18 04:54:35 +00:00
Hans Wennborg
fc0acdb90f Test commit.
llvm-svn: 132558
2011-06-03 17:15:37 +00:00
Jay Foad
0d5ca4cf44 Don't include Operator.h from InstrTypes.h.
llvm-svn: 129271
2011-04-11 09:35:34 +00:00
Dan Gohman
295ba3ab26 Move Value::getUnderlyingObject to be a standalone
function so that it can live in Analysis instead of
VMCore.

llvm-svn: 121885
2010-12-15 20:02:24 +00:00
Dan Gohman
befe1767ff Reapply r116831 and r116839, converting AliasAnalysis to use
uint64_t, plus fixes for places I missed before.

llvm-svn: 116875
2010-10-19 22:54:46 +00:00
Dan Gohman
0d7d3faf8e Move FindAvailableLoadedValue isSafeToLoadUnconditionally out of
lib/Transforms/Utils and into lib/Analysis so that Analysis passes
can use them.

llvm-svn: 104949
2010-05-28 16:19:17 +00:00