Fix for #19973

[kvanberendonck]

Summary

Fix for #19973

http://llvm.org/bugs/show_bug.cgi?id=19973

Fails 1 test, which seems to be a test checking for the behaviour I don't think we want.

[kvanberendonck]

Sorry for the whitespace. Originally I had the constant offset check there, but I had to move it because shouldMergeGEP wasn't doing the right thing.

[kvanberendonck]

Fixed junk

[kvanberendonck]

Got to write a regression test and remove the broken test case.

[kvanberendonck]

Now removed the test which was failing (testing for incorrect behaviour) and added 2 new regression test cases -- one of which currently fails (test_gep_canon_2_add). I'm working on getting it right again.

[kvanberendonck]

There may be a constant floating problem in the direction we float constants outlined by the bottom tests. It's hard to pull constants out of stacks of arithmetic because they are moved to the bottom(?). It would be more efficient if they were floated to the top (i.e. closer to the things that use them), maybe.

[chandlerc]

This patch has gone quite stale. I don't really understand why this particular change is the right one, I think it'll need some comments or other clarification in that respect before we accept it... If you end up wanting to continue working on this, try to do that, update the patch, and re-add me so I can take a look.

include/llvm/IR/Operator.h

     }
     return true;
   }
+  
+  /// hasAConstantIndex - Returns true if any of the indices of this GEP are
+  /// constant integers.
+  bool hasAConstantIndex() const {
+    for (const_op_iterator I = idx_begin(), E = idx_end(); I != E; ++I) {
+      if (isa<ConstantInt>(I))
+        return true;
+    }
+    return false;
+  }
 
   /// hasAllConstantIndices - Return true if all of the indices of this GEP are
   /// constant integers.  If so, the result pointer and the first operand have

lib/Transforms/InstCombine/InstructionCombining.cpp

   if (GEP.hasAllZeroIndices() && !Src.hasAllZeroIndices() &&
       !Src.hasOneUse())
     return false;
+  
   return true;
 }
 

         // normalized.
         if (SO1->getType() != GO1->getType())
           return nullptr;
+          
+        // Is it worth summing the offsets?
+        if (!isa<ConstantInt>(SO1))
+          return nullptr;
+        
         Sum = Builder->CreateAdd(SO1, GO1, PtrOp->getName()+".sum");
       }