[PATCH] D30767: Updates branch_weights annotation for call instructions during inlining.

[Easwaran Raman via Phabricator via llvm-commits]

eraman added inline comments.


================
Comment at: lib/IR/Instruction.cpp:694
+    Val *= APInt(128, S);
+    Weights.push_back(Val.udiv(APInt(128, T)).getLimitedValue());
+  }
----------------
Perhaps it is not clear above, but SaturatingMultiply may be sufficient in your case. I'm ok with using APInt as well as in the most common case (multiplication result fits within 64 bits), the division is not expensive. Perhaps add a comment above the udiv stating that this could potentially be expensive, but most likely the product is going to fit within 64 bits. 


================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:1450
+          // No need to update the callsite if it is pruned during inlining.
+          if (VMap.count(CI))
+            CI->updateProfWeight(CalleeEntryCount.getValue() - CallCount,
----------------
The check above is not needed. After all, if the block is reachable in the call context, all instructions inside are reachable too. But there is one tricky case. Let's say that a call has been simplified into a value (this can happen during pruning), Should you update the call count or not? If you should not update, then the check above should be isa<CallInst>(VMap.Count(CI)). I think updating is the right thing here though.


https://reviews.llvm.org/D30767