[PATCH] D27518: Moving isComplex decision to TTI
Michael Kuperstein via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 7 13:33:03 PST 2016
mkuper added a comment.
I have some comments on the patch, but on a higher level, I'm not sure this is the right way to do this.
The only place that'll call this new TTI hook will be isLikelyComplexAddressComputation(), and the only thing we do with the result of isLikelyComplexAddressComputation() is feed it into a different TTI hook - TTI.getAddressComputationCost().
Perhaps we can enhance getAddressComputationCost() instead of adding another hook?
================
Comment at: include/llvm/Analysis/TargetTransformInfo.h:210
+ /// \p Stride holds a pointer to the Stride value in bytes if it is a compile
+ /// time constant, or nullptr otherwise.
+ bool isComplexStridedAddressComputation(const APInt* Stride) const;
----------------
Extra space.
================
Comment at: include/llvm/Analysis/TargetTransformInfo.h:211
+ /// time constant, or nullptr otherwise.
+ bool isComplexStridedAddressComputation(const APInt* Stride) const;
+
----------------
Maybe "Expensive" makes more sense than "Complex" here?
================
Comment at: lib/Target/X86/X86TargetTransformInfo.cpp:1480
+ // compile time constant stride we dont see any significant address
+ // computation cost.
+ return false;
----------------
What do you mean by "we don't see any significant address computation cost"? Can you give an example?
================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:6618
if (!C)
- return true;
-
- const APInt &APStepVal = C->getAPInt();
-
- // Huge step value - give up.
- if (APStepVal.getBitWidth() > 64)
- return true;
+ return TTI.isComplexStridedAddressComputation(NULL);
----------------
NULL -> nullptr
Also, this looks a bit dirty. Maybe use Optional<>?
https://reviews.llvm.org/D27518
More information about the llvm-commits
mailing list