[PATCH] D18679: Port demanded-bits to the new pass manager
Michael Kuperstein via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 18 12:12:08 PDT 2016
mkuper added a comment.
Thanks, Chandler.
================
Comment at: include/llvm/Analysis/DemandedBits.h:39
@@ -37,4 +38,3 @@
-struct DemandedBits : public FunctionPass {
- static char ID; // Pass identification, replacement for typeid
- DemandedBits();
+class DemandedBitsResult {
+public:
----------------
chandlerc wrote:
> I would just call this "DemandedBits". In many cases, the original name was already a useful noun describing the results and query API and not really anything to do with the analysis.
Right, I just thought it may be confusing to reuse the old name of the pass for the result.
Will change.
================
Comment at: include/llvm/Analysis/DemandedBits.h:71
@@ -69,1 +70,3 @@
+class DemandedBitsLegacyPass : public FunctionPass {
+private:
----------------
chandlerc wrote:
> I've been calling these "...WrapperPass" to signify that they actually contain the result object for the legacy PM.
>
> DomTree and LoopInfo I think are reasonable examples here.
Ack.
================
Comment at: include/llvm/Analysis/DemandedBits.h:73
@@ +72,3 @@
+private:
+ mutable Optional<DemandedBitsResult> DB;
+public:
----------------
chandlerc wrote:
> Why mutable?
Because the pass is lazy, print() has to run performAnalysis().
Since print() is an overloaded const method, the old version had:
```
// This is gross. But the alternative is making all the state mutable
// just because of this one debugging method.
const_cast<DemandedBits*>(this)->performAnalysis();
```
I thought having the result mutable was nicer, but I'm ok with keeping the const_cast if you prefer it.
Or anything else which is nicer than either.
http://reviews.llvm.org/D18679
More information about the llvm-commits
mailing list