[PATCH] D13551: [LoopVectorize] Shrink integer operations into the smallest type possible

[silviu.baranga@arm.com via llvm-commits]

sbaranga added a comment.

Hi James,

I generally really like the idea of using this DemandedBits analysis here. Otherwise, I have a few comments (inline).

Do you see any changes to lnt/spec performance with this patch?

Thanks,
Silviu


================
Comment at: lib/Analysis/VectorUtils.cpp:441
@@ +440,3 @@
+
+DenseMap<Instruction*, uint64_t> llvm::computeMinimumValueSizes(
+  ArrayRef<BasicBlock*> Blocks, DemandedBits &DB,
----------------
Returning possibly large structs is not ideal. Would it be better for this function to take a reference to a map instead?

================
Comment at: lib/Analysis/VectorUtils.cpp:549
@@ +548,3 @@
+    
+    uint64_t MinBW = 64 - llvm::countLeadingZeros(LeaderDemandedBits);
+    // Round up to a power of 2
----------------
Should be sizeof(LeaderDemandedBits) instead of 64.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3146
@@ +3145,3 @@
+  // later and will remove any ext/trunc pairs.
+  //
+  for (auto &KV : MinBWs) {
----------------
Is there any way of doing this without leaving the ext/trunc pairs?

Maybe instead of generating a trunc look if we're doing it for an ext and if so use the ext operand instead.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3783
@@ -3653,2 +3782,3 @@
       VectorParts &B = getVectorValue(it->getOperand(1));
+
       for (unsigned Part = 0; Part < UF; ++Part) {
----------------
Stray whitespace change?

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:5303
@@ -5169,3 +5302,3 @@
   case Instruction::FCmp: {
     Type *ValTy = I->getOperand(0)->getType();
     VectorTy = ToVectorTy(ValTy, VF);
----------------
I think you wouldn't get the correct cost here (for ICmp)?


Repository:
  rL LLVM

http://reviews.llvm.org/D13551