[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