[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