Bug in pass 'ipsccp' on function attribute 'argmemonly'?

Hi all,

Let’s see an example (from Alexandre Isoard) first:

Hi Fangqing,

It looks like that after -deadargelim foo() should not have an argmemonly attribute. If you also run Attributor here, you get the expected result: https://godbolt.org/z/8hcG9c

Unfortunately, Attributor doesn’t yet run by default.

-Stefan

Hi, Fangqing,

Yes, this seems like a bug. Can you please file a bug report at ? If you need someone else to do this on your behalf, please let us know.

-Hal

Thank you Hal and Stefan!

Bug report is filed: https://bugs.llvm.org/show_bug.cgi?id=46717

And Stefan,
I think ‘attributor’ is a really nice pass, and can infer more precise and useful attributes, which may give more opportunities for optimization.
But we shouldn’t depend on ‘attributor’ to correct wrong function attributes, because we cannot run ‘attributor’ after every pass, right?

Thanks,
Fangqing

I think here, I would fix ipsccp.

It transform an access of a function argument based on that argument into an access of a function argument not based on that argument.

While one could argue it is still argmemonly (the documentation is ambiguous on that) it makes it significantly harder to trace that correlation for deadargelim or any form of alias analysis.

In general I wonder if we would not benefit from an attribute that list the globals that a function may access, and having argmemonly include those globals in what it consider “arg”.

Thank you Hal and Stefan!

Bug report is filed: https://bugs.llvm.org/show_bug.cgi?id=46717

And Stefan,
I think 'attributor' is a really nice pass, and can infer more precise and useful attributes, which may give more opportunities for optimization.
But we shouldn't depend on 'attributor' to correct wrong function attributes, because we cannot run 'attributor' after every pass, right?

Correct. Each pass must be correct on its own. We need to fix ipsccp.

-Hal

Thank you Hal and Stefan!

Bug report is filed: https://bugs.llvm.org/show_bug.cgi?id=46717

And Stefan,
I think 'attributor' is a really nice pass, and can infer more precise and useful attributes, which may give more opportunities for optimization.
But we shouldn't depend on 'attributor' to correct wrong function attributes, because we cannot run 'attributor' after every pass, right?

Correct. Each pass must be correct on its own. We need to fix ipsccp.

I don't think that was ever questioned :wink:

I think the comment about the Attributor was more to show that this behavior

is not the same there which is yet another good indicator this is a bug.

~ Johannes

Note that there is the same issue in ipconstprop (on the same example) for the same reason. (thanks @Lin-Ya Yu)

Thanks for the report. I put up a fix
https://reviews.llvm.org/D84432