[PATCH] D59149: [LV] move useEmulatedMaskMemRefHack() functionality to TTI.

[Renato Golin via Phabricator via llvm-commits]

rengolin added a comment.

The concept of "hacked" is lost when you move up to TTI. I'd change the logic to reflect that this is making it "prohibitively expensive" instead of "hacked value".

It doesn't need to change much, just use the "override value" pattern:

  Cost = someComputation();
  // Target can override mask cost (ex. when it's prohibitively expensive)
  EmulatedCost = TTI.getEmulatedMaskMemRefCost(...);
  if (EmulatedCost)
    Cost = EmulatedCost;

Also, that's perhaps not the right place to add actual costs. Moving up the boolean function as is would make more sense, but the "hacked cost" would still remain.

Can't you move this to the cost model?



================
Comment at: llvm/include/llvm/Analysis/TargetTransformInfoImpl.h:270
+                         (isa<StoreInst>(Inst) && NumPredStores > Threshold);
+    return useHackedCost ? 3000000 : 0;
+  }
----------------
markus wrote:
> This value seems a bit arbitrary. Could it not be e.g. INT_MAX instead if it is supposed to represent something infinitely expensive?
Costs will be added to others, INT_MAX would wrap.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5488
 
-    if (useEmulatedMaskMemRefHack(I))
+    if (int HackedCost = TTI.getEmulatedMaskMemRefCost(
+            I, NumPredStores, NumberOfStoresToPredicate))
----------------
This if doesn't need the Cost above, so you can avoid the division by moving it up to the beginning of the block.

Nit: you can just return `HackedCost` instead of assigning, but that will depend on the new pattern.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59149