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

[Sean Silva via llvm-commits]

On Fri, Jun 24, 2016 at 3:30 PM, Michael Kuperstein <mkuper at google.com>
wrote:

> mkuper added a comment.
>
> Thanks, David and Davide.
>
>
> ================
> Comment at: lib/Transforms/Scalar/Float2Int.cpp:73
> @@ -88,1 +72,3 @@
> +                      false)
>  INITIALIZE_PASS_DEPENDENCY(GlobalsAAWrapperPass)
> +INITIALIZE_PASS_END(Float2IntLegacyPass, "float2int", "Float to int",
> false,
> ----------------
> davide wrote:
> > This is not your bug, but still worth mentioning. This pass preserves
> globalsAA, doesn't depend on it. In other words, this can go away.
> Right, I noticed that too, but forgot about it. I'll remove it.
>
> ================
> Comment at: lib/Transforms/Scalar/Float2Int.cpp:537
> @@ +536,3 @@
> +PreservedAnalyses Float2IntPass::run(Function &F, FunctionAnalysisManager
> &) {
> +  //FIXME: skipFunction is not currently supported in the new PM.
> +  if (!Impl.runImpl(F))
> ----------------
> davide wrote:
> > davidxl wrote:
> > > This is a good point. Is there a bug tracking this missing
> functionality?
> > I'd remove the FIXME. No other pass mentions it, and I'm not sure if
> there's active work on this.
> You're probably right.
> I remember some talk about this possibly being handled by the PM, so that
> run() methods won't even be called when the function should be skipped.
> I'll remove the FIXME, thanks.
>

I have filed: https://llvm.org/bugs/show_bug.cgi?id=28316
And added it as a dependency for https://llvm.org/bugs/show_bug.cgi?id=28315
(I just created that and will be filling it in shortly)

-- Sean Silva


>
> ================
> Comment at: lib/Transforms/Scalar/Float2Int.cpp:541
> @@ +540,3 @@
> +  else {
> +    //FIXME: setPreservesCFG is not currently supported in the new PM.
> +    PreservedAnalyses PA;
> ----------------
> 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.
>
>
> http://reviews.llvm.org/D21704
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160626/08e108af/attachment.html>