[llvm-commits] PATCH: Add support for tracking pairs of pointers with a constant offset to inline cost estimation

[Nick Lewycky]

Chandler Carruth wrote:
> Hello folks!
>
> This is an early patch, it still has some rough edges. In particular, I
> want to clean up the body of the loop which looks for operations to
> simplify in the function body, but I wanted to start showing people
> where I'm going here and get any feedback sooner rather than later.

+  // Check which direction the expect result points so that we know how 
to flip
+  // the value across bitwise operations.
    bool Result = ICI->isTrueWhenEqual();

We don't flip the value, all instructions we recursively visit must all 
return Result. If they don't, we don't visit them.

+void InlineCostAnalyzer::FunctionInfo::countCodeReductionForPointerPair(
+    const CodeMetrics &Metrics, DenseMap<Value *, unsigned> &PointerArgs,
+    Value *V, unsigned ArgIdx) {
+  SmallVector<Value *, 4> Worklist;
+  Worklist.push_back(V);
+  do {
+    Value *V = Worklist.pop_back_val();
+    for (Value::use_iterator UI = V->use_begin(), E = V->use_end();
+         UI != E; ++UI){

Missing space before {

+      if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(I)) {
+        // If the GEP has variable indices, we won't be able to do much 
with it.
+        if (!GEP->hasAllConstantIndices())
+          continue;

inbounds should be sufficient as well.

Also, this function should handle SelectInst. Something like:

   // Track through a select between two related pointers.
   if (SelectInst *SI = dyn_cast<SelectInst>(I)) {
     Value *ThisArg = SI->getTrueValue();
     Value *OtherArg = SI->getFalseValue();
     if (ThisArg != V)
       std::swap(ThisArg, OtherArg);
     if (ThisArg != V)
       continue;

     DenseMap<Value *, unsigned>::const_iterator ArgIt =
         PointerArgs.find(OtherArg);
     if (ArgIt != PointerArgs.end() &&
         *ArgIt == ArgIdx) {
       PointerArgs[SI] = ArgIdx;
       Worklist.push_back(SI);
       continue;
     }
   }

.

+static bool isPointerPairConstantRelated(Value *X, Value *Y) {
+  // Unroll both sides until we hit a fixed point where the pointer 
value "came
+  // from". If the two values are constant-related, they will halt at 
the same
+  // fixed point.
+  while (stripConstantGEP(X) || stripBitCast(X))
+    ;
+  while (stripConstantGEP(Y) || stripBitCast(Y))
+    ;
+
+  return X == Y;
+}

This is nearly X->stripPointerCasts() == Y->stripPointerCasts(), but 
includes non-zero GEPs. I suggest taking 
lib/Analysis/InstructionSimplify.cpp's stripPointerAdjustments() and 
either make it a method on Value* like stripPointerCasts, or else 
somewhere publicly visible in libAnalysis.

Nick

> The motivation for these heroics (and I recognize that these are
> heroics) is to handle cases like:
>
> template <typename IteratorT>
> foo my_algorithm(IteratorT first, IteratorT last) {
>    if (std::distance(first, last) < 42) {
>      // Algorithm A
>    } else {
>      // Algorithm B
>    }
> }
>
> Without this patch, the inline cost will be assumed to be the total size
> of the function, even when we call it like so:
>
> void test(int *ptr) {
>    foo bar = my_algorithm(ptr, ptr + 13);
> }
>
> Here, the arguments are a known constant offset, and yet we won't see
> that, or see that only one half of the branch in my_algorithm is going
> to survive. This causes missed optimizations every time we use an STL
> algorithm on a fixed buffer (usually a stack object).
>
> The patch tries to track these correlated pointer values, and detect
> when they're used in a way that will simplify. Initial testing shows
> that it works, but I haven't yet reduced my big test cases into nice
> small feature tests for the test suite. Clearly, that has to happen
> before check-in.
>
>
> Let me know your thoughts! I'll post a new patch as I clean it up, and
> I'll also post numbers as I have them on the impact this gives.
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits