[PATCH] D135808: [LoopInterchange] Correcting the profitability checking for vectorization

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopInterchange.cpp:1159-1160
     LLVM_DEBUG(dbgs() << "Cost = " << Cost << "\n");
     if (Cost < -LoopInterchangeCostThreshold)
       return true;
   }
----------------
The structure suggested in the LoopWG call. 

The more general pattern/refactoring would be:
```
  std::optional<bool> shouldInterchange = isProfitableAccordingLoopCacheAnalysis(..);
  if (shouldInterchange.has_value()) 
    return shouldInterchange.get_value();

  shouldInterchange = isProfitableAccordingInstrOrderCost(..);
  if (shouldInterchange.has_value()) 
    return shouldInterchange.get_value();

  shouldInterchange = isProfitableForVectorization(..);
  if (shouldInterchange.has_value()) 
    return shouldInterchange.get_value();

  emitOptimizationRemark("Don't know");
```

However, this changes when the `emitOptimizationRemark` is called. If we would not want to change this, it would be (which corresponds to the current structure but with refactoring):
```
  std::optional<bool> shouldInterchange = isProfitableAccordingLoopCacheAnalysis(..);
  if (!shouldInterchange.has_value()) {
    shouldInterchange = isProfitableAccordingInstrOrderCost(..);
    if (!shouldInterchange.has_value()) {
       shouldInterchange = isProfitableForVectorization(..);
    }
  }

  if (!shouldInterchange.has_value())
    emitOptimizationRemark("Don't know");
  else if (!shouldInterchange.get_value())
    emitOptimizationRemark("Profitability heuristic indicates this loop is good as-is");
```
although I would prefer the former over the nested if-else chain and instead emit different optimization remarks for each of the heuristic that indicates that/why the loops should (NOT) be interchanged.



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135808/new/

https://reviews.llvm.org/D135808