[PATCH] D140851: [InstCombine]: Add cases for assume (X & Y != {0|Y})

[Nikita Popov via Phabricator via llvm-commits]

nikic accepted this revision.
nikic added a comment.
This revision is now accepted and ready to land.

LGTM

In D140851#4040862 <https://reviews.llvm.org/D140851#4040862>, @goldstein.w.n wrote:

> Also, if you have a moment about an unrelated thing. I'm working on a patch
> for `SimplifyLibCalls` to see if there is a previous dominating `slen = strlen(s)` call
> that will allow transformation for `str*(s, ...)` -> `mem*(s, ..., slen)` even if the string is non-constant.
> To do this, it needs to check that between the `strlen` call and lib-call the memory
> `*s` can't have been clobbered. I think the "right" way to do this is `memoryssa` but
> it seems ridiculous (and overly consuming of compile time) to add `memoryssa` to all
> of `instcombine` just for this one relatively unimportant pass. I saw we have `AAResults`
> already computed so started with that, but it only works on a single basic-block (and since
> we don't have `postdom` tree there doesn't seem to be any cheap way to collect all possible
> affected basic-blocks between the `strlen` call and lib-call. My thought for how to handle
> this was either:
>
> 1. Bite the bullet and use `memoryssa` throughout `instcombine`.

Extremely unlikely. Both because of compile-time impact, and because this would add implementation complexity to all other transforms, that now need to keep MSSA up to date.

> 2. Only handle `strlen` calls in the same basic-block as the lib-call and use `AAResults`

This is possible -- probably easiest if it covers your motivating cases.

> 3. Add a file to "canonicalize" lib calls before EarlyCSE by making ALL `str*(s,...)` functions `mem*(s, ..., strlen(s))`, let `EarlyCSE` remove all duplicate `strlen` calls, then make the `SimplifyLibCall` in `instcombine` replace `mem*(s, ..., strlen(s))` with the `str*` equivilent.

Handling this in EarlyCSE/GVN does seem most principled. I'm not sure it's necessary to actually perform the conversion from str to mem+strlen in advance -- shouldn't we be able to look up whether we have an available strlen call, and perform the transform from str to mem if we do? Of course, it would be a bit of an awkward special case...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140851