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

[Michael Kuperstein via llvm-commits]

mkuper added inline comments.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3327
@@ -3321,1 +3326,3 @@
+      if (Predicate)
+        Cost += TTI.getCFInstrCost(Instruction::PHI);
       Cost += TTI.getVectorInstrCost(Instruction::InsertElement, Ty, I);
----------------
gilr wrote:
> mkuper wrote:
> > I'm not entirely sure what "the cost of a phi" means, especially without a type.
> > 
> > Also, I don't believe any in-tree target actually assigns a non-zero to PHIs right now. So if you actually want a meaningful cost here for, say, x86, you may want to look into the cost model as well...
> So IIRC I took it from CostModelAnalysis::getInstructionCost(), but I'll remove it if it makes no sense.
I'm not sure what makes sense here, to be honest.
What cost, in the final generated code, are you trying to account for? An extra register copy?

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:4251
@@ +4250,3 @@
+    case Instruction::SRem:
+    case Instruction::FRem:
+      // Scalarize if block needs predication, otherwise fallthrough.
----------------
gilr wrote:
> mkuper wrote:
> > FRem looks out of place. Did you mean URem?
> Yes.
Too bad there wasn't a test that would have failed because of this. ;-) 


https://reviews.llvm.org/D22918