[llvm-branch-commits] [polly] r183379 - fix insertion of values in BBMap

[Tobias Grosser]

Author: grosser
Date: Wed Jun  5 21:48:30 2013
New Revision: 183379

URL: http://llvm.org/viewvc/llvm-project?rev=183379&view=rev
Log:
fix insertion of values in BBMap

In GDB when "step" through generateScalarLoad and "finish" the call, the
returned value is non NULL, however when printing the value contained in
BBMap[Load] after this stmt:

  BBMap[Load] = generateScalarLoad(...);

the value in BBMap[Load] is NULL, and the BBMap.count(Load) is 1.

The only intuitive idea that I have to explain this behavior is that we are
playing with the undefined behavior of eval order of the params for the function
standing for "BBMap[Load] = generateScalarLoad()". "BBMap[Load] = " may be
executed before generateScalarLoad is called.

Here are some other possible explanations from Will Dietz <w at wdtz.org>:

The error is likely due to BBMap[Load] being evaluated first (creating
a {Load -> uninitialized } entry in the DenseMap), then
generateScalarLoad eventually accesses the same element and finds it
to be NULL (DenseMap[Old])..  Offhand I'm not sure if this is
guaranteed to be NULL or if it's uninitialized and happens to be NULL.

The same issue can also go wrong in an even worse way: the second
DenseMap access can trigger a rehash and *invalidate* the an earlier
evaluated expression (for example LHS of the assignment), leading to a
crash when performing the assignment store.

Merged from: https://llvm.org/svn/llvm-project/polly/trunk@182655

Modified:
    polly/branches/release_33/lib/CodeGen/BlockGenerators.cpp

Modified: polly/branches/release_33/lib/CodeGen/BlockGenerators.cpp
URL: http://llvm.org/viewvc/llvm-project/polly/branches/release_33/lib/CodeGen/BlockGenerators.cpp?rev=183379&r1=183378&r2=183379&view=diff
==============================================================================
--- polly/branches/release_33/lib/CodeGen/BlockGenerators.cpp (original)
+++ polly/branches/release_33/lib/CodeGen/BlockGenerators.cpp Wed Jun  5 21:48:30 2013
@@ -185,6 +185,7 @@ Value *BlockGenerator::getNewValue(const
   }
 
   if (BBMap.count(Old)) {
+    assert(BBMap[Old] && "BBMap[Old] should not be NULL!");
     return BBMap[Old];
   }
 
@@ -354,12 +355,14 @@ void BlockGenerator::copyInstruction(con
     return;
 
   if (const LoadInst *Load = dyn_cast<LoadInst>(Inst)) {
-    BBMap[Load] = generateScalarLoad(Load, BBMap, GlobalMap, LTS);
+    Value *NewLoad = generateScalarLoad(Load, BBMap, GlobalMap, LTS);
+    BBMap[Load] = NewLoad;
     return;
   }
 
   if (const StoreInst *Store = dyn_cast<StoreInst>(Inst)) {
-    BBMap[Store] = generateScalarStore(Store, BBMap, GlobalMap, LTS);
+    Value *NewStore = generateScalarStore(Store, BBMap, GlobalMap, LTS);
+    BBMap[Store] = NewStore;
     return;
   }