[PATCH] Using Masked Load / Store intrinsics in Loop Vectorizer

[Michael Zolotukhin]

Hi Elena,

Please see my comments inline.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:5386-5387
@@ +5385,4 @@
+          !SI->getParent()->getSinglePredecessor()) {
+        if (canPredicateStore(SI->getValueOperand()->getType(),
+                                  SI->getPointerOperand())) {
+          MaskedOp.insert(SI);
----------------
delena wrote:
> mzolotukhin wrote:
> > Could this condition be simplified? E.g. we can store result of canPredicateStore(..) in a bool variable and use it here.
> > Also, I think it might be incorrect - shouldn't we insert SI to MaskedOp even if NumPredStores hasn't exceeded NumberOfStoresToPredicate?
>  If  NumPredStores hasn't exceeded NumberOfStoresToPredicate I'll not generate  masked store. I want to keep the current behavior.
> 
> Something like this?
>       bool isSafePtr = (SafePtrs.count(SI->getPointerOperand()) !=0);
>       bool isSinglePredecessor = SI->getParent()->getSinglePredecessor();
>       
>       if (++NumPredStores > NumberOfStoresToPredicate || !isSafePtr ||
>           !isSinglePredecessor) {
>         // Build a masked store if it is legal for the target, otherwise scalarize
>         // the block.
>         bool isLegalMaskedOp = canPredicateStore(SI->getValueOperand()->getType(),
>                                                  SI->getPointerOperand());
>         if (isLegalMaskedOp) {
>           --NumPredStores;
>           MaskedOp.insert(SI);
>           continue;
>         }
>         return false;
>       }
> 
Actually, I wanted to get rid of ++NumPredStores, followed by --NumPredStores.
I'd rewrite it like this:

```
if (!isSafePtr || !isSinglePredecessor) {
    if (isLegalMaskedOp) {
        // This store will be masked, no predication is needed for it.
        MaskedOp.insert(SI);
        continue;
    } else {
        // This store needs to be predicated.
        if (++NumPredStores > NumberOfStoresToPredicate)
            return false;
    }
}
```
Also, I wonder if we actually need a 'continue' statement in if(isLegalMaskedOp) - do you intentionally skip subsequent checks (e.g. it->mayThrow) for this case?

http://reviews.llvm.org/D6527