[PATCH] D89050: Add support for !noundef metatdata on loads

Mon Oct 12 08:17:41 PDT 2020

jdoerfert added a comment.

In D89050#2324427 <https://reviews.llvm.org/D89050#2324427>, @aqjune wrote:

> I made D89219 <https://reviews.llvm.org/D89219> that updates relevant functions to exploit `llvm.assume`'s `noundef` operand bundle.

Thx!

> BTW, given `v = op; call llvm.assume()["noundef"(v)]`, if we want to move `v` across the assume call, the call should be appropriately moved with `v`. I wonder whether there is a handy way to do this, since things get complicated when the assume has several operand bundles that are not using `v`.

Operand bundles are unknown uses. You should not be able to break things. Hoisting `v` does and should not impact the operand bundles. If you want to sink `v` you need to know about "droppable uses" and "drop them", SROA knows about them nowadays for example.

> What about leaving `call llvm.assume` whenever the `load !noundef` is moved to somewhere else? I guess this is what LLVM is doing as well.

If `--enable-knowledge-retention` is set, we should certainly do that if we would otherwise strip the metadata (not only for noundef).

> `!noundef` is a syntactic sugar of `llvm.assume()["noundef"(..)]`, but seems helpful for the brevity and size of the bitcode.

I agree, especially if there is no `llvm.assume` available in the context to attach the information. If there is, I doubt there is overhead. My idea for the future is that we will have enough `llvm.assume`s such that this would not be a problem, though I won't block the metadata because of this. (Maybe it was wrong to start the discussion in this review then)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89050