[PATCH] D21183: Better selection of common base address in constant hoisting
James Molloy via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 8 05:45:27 PDT 2016
jmolloy added inline comments.
================
Comment at: include/llvm/Analysis/TargetTransformInfo.h:413
@@ +412,3 @@
+
+ /// \brief Return the expected cost for the given integer when optimising
+ /// for size. This is different than the other integer immediate cost
----------------
Having this hook be context-free will hinder targets implementing it usefully. It should take the same arguments as getIntImmCost(), so that targets can give a contextual answer if they wish.
================
Comment at: lib/Transforms/Scalar/ConstantHoisting.cpp:288
@@ -287,1 +287,3 @@
+// This helper function is necessary to deal with values that have differend
+// bit widths (APInt Operator- does not like that). If the value cannot be
----------------
"different"
================
Comment at: lib/Transforms/Scalar/ConstantHoisting.cpp:341
@@ +340,3 @@
+ DEBUG(dbgs() << "= Constant: " << ConstCand->ConstInt->getValue() << "\n");
+ for (auto User = ConstCand->Uses.begin(); User != ConstCand->Uses.end(); ++User) {
+ auto Value = ConstCand->ConstInt->getValue();
----------------
You can use a range-for loop here.
================
Comment at: lib/Transforms/Scalar/ConstantHoisting.cpp:342
@@ +341,3 @@
+ for (auto User = ConstCand->Uses.begin(); User != ConstCand->Uses.end(); ++User) {
+ auto Value = ConstCand->ConstInt->getValue();
+ Cost += TTI->getIntImmCost(User->Inst->getOpcode(), User->OpndIdx,
----------------
This should be hoisted out of the inner loop as it is it invariant.
================
Comment at: lib/Transforms/Scalar/ConstantHoisting.cpp:347
@@ +346,3 @@
+ for (auto C2 = S; C2 != E; ++C2) {
+ llvm::Optional<APInt> Diff = calculateOffsetDiff(
+ C2->ConstInt->getValue(),
----------------
The formatting looks off here - have you used clang-format?
http://reviews.llvm.org/D21183
More information about the llvm-commits
mailing list