[PATCH] D76886: [InlineFunction] Disable emission of alignment assumptions by default

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D76886#1946476 <https://reviews.llvm.org/D76886#1946476>, @hfinkel wrote:

> I don't object to doing this, but...
>
> > After looking at IR diffs a bit more, one difference I noticed is that jump threading is not being performed in some places. In this case the reason is not multi-use, but cost heuristics based on instruction counts. The alignment assumption adds 4 extra instructions, and if the block duplication threshold is at 6 instructions, that can easily make a difference.
>
> This an independent problem that we might fix first. Jump threading should be collecting and ignoring ephemeral values for it's heuristic. If it's not doing that, please fix that first and then reevaluate.


Given the ongoing work to convert assumes to operand bundles, which would reduce ignoring of ephemeral values to ignoring the assume itself, I don't think it makes sense to do such a change at this time. Ignoring ephemeral values is not cheap, and we have numerous places that use limited length instruction scans as a cheap heuristic. I don't want to start computing ephemeral values every time InstCombine does a backwards scan from a load or store.

I'm only using JumpThreading as an illustrative example for a test case here. As you well know, JumpThreading is neither the only nor the most important issue when it comes to handling of assumes. As things stand right now, fixing any particular place is not going to change anything about the big picture. And of course, it will do nothing about compile-time impact.

That said, we're already forced to disable this in Rust anyway, so I'm not particularly invested in making this change -- it makes no difference to us. I'm happy to abandon this revision if it does not seem sufficiently beneficial on its own.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76886/new/

https://reviews.llvm.org/D76886