[PATCH] D22918: [Loop Vectorizer] Support predication of div/rem

[Matthew Simpson via llvm-commits]

mssimpso added inline comments.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:4135-4138
@@ +4134,6 @@
+        IncomingFalse = IEI->getOperand(0); // the unmodified vector
+      } else {
+        // Phi node will be created for the scalar predicated instruction.
+        IncomingTrue = &*I;
+        IncomingFalse = UndefValue::get(I->getType());
+      }
----------------
gilr wrote:
> anemet wrote:
> > Is there a test for the non-insertelt case?
> No. We currently always create an insertelement on scalarization. I added support for the non-insertelement case for completeness, since predication is done separately from scalarization.
> IIUC this case will need to be supported if & when [[ https://reviews.llvm.org/D23169 | Matthew's patch ]] is committed, but for now it's really FFU.
> I'll replace this case with an assertion for this patch and leave it to Matthew to resurrect in his patch as needed.
> 
Gil,

For the non-insert cases that might arise with my patch, I think it would be better to leave the case here (don't add an assert), but include a test with this patch that would break with the other patch. Does that make sense? That will keep the patches better self-contained and prevent us from having to revisit this.

You will need a test where the div only feeds an instruction that will also be scalar (like a GEP). If this patch lands before the other one, it will be cleaner if in the other we just have to change the test. Presumably, that would involve replacing the PHI for the inserts with a PHI for the predicated instruction, since the inserts will no longer be there? If the other patch lands first, there will be no issue since you'll already have a test for the non-insert case.


https://reviews.llvm.org/D22918