[PATCH] D21704: [PM] Port float2int to the new pass manager

Davide Italiano via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 24 15:35:15 PDT 2016


davide added a comment.

This one looks good modulo two small comments.


================
Comment at: lib/Transforms/Scalar/Float2Int.cpp:526
@@ +525,3 @@
+
+bool Float2IntLegacyPass::runOnFunction(Function &F) {
+  if (skipFunction(F))
----------------
This function can be inlined (line 60), no?
`   bool runOnFunction(Function &F) override;`


================
Comment at: lib/Transforms/Scalar/Float2Int.cpp:541
@@ +540,3 @@
+  else {
+    //FIXME: setPreservesCFG is not currently supported in the new PM.
+    PreservedAnalyses PA;
----------------
mkuper wrote:
> davide wrote:
> > davidxl wrote:
> > > same here.
> > I don't think there's a bug tracker for this (probably there should be one tho?). 
> > As a general comment, this needs an understanding which passes actually now 'preserve the CFG' (whatever that means, we need to agree on a sane/clear semantic) so I'm not entirely sure the changes here will be NFC. 
> I don't know about a bug tracker, but there are FIXMEs for this in many ported passes. I think it's good to have these fixmes regardless of what exactly happens in the future, simply to serve as (another) marker for passes where it makes *some* sense.
Yes a comment here makes sense, thanks! Can you please copy'n'paste from any of the passes I already ported (so that we have the same comment everywhere and it's easier to grep and/or locate when we want to replace)


http://reviews.llvm.org/D21704





More information about the llvm-commits mailing list