[PATCH] D97735: [Globals] Treat nobuiltin fns as maybe-derefined.

[Richard Smith - zygoloid via Phabricator via llvm-commits]

rsmith added a comment.

In D97735#2595750 <https://reviews.llvm.org/D97735#2595750>, @jdoerfert wrote:

> I'm not thrilled to pretend they may be derefinded but it seems practical and sufficient. Fine with me.

I think it's feasible to argue that this really is a derefinement issue -- or at least the dual of one. In this case, we have two different notional definitions of the function:

1. LLVM's idealized model of the builtin function.
2. The actual implementation.

... where (2) is (ideally) a refinement of (1). In a normal derefinement bug, we can't use positive properties of (2) because we might end up picking a different refinement of (1) as the actual definition. In this case, we can't use negative properties of (1) (such as "parameter is noundef") after refining to (2) because those properties might not be true of (2). I suppose one way to get a bit more comfortable with calling this derefinement would be to argue that when the optimizer makes transformations based on known builtin semantics, what it's notionally doing is replacing the function definition with the idealized builtin definition (which is a derefinement step, at least if we've already seen the real definition), performing analysis / transformation with that derefined definition, then re-refining back to the actual definition.



================
Comment at: llvm/include/llvm/IR/GlobalValue.h:147
+      // derefined.
+      return isInterposable() || isNobuiltinFnDef();
     }
----------------
I don't think `nobuiltin` is relevant; I think what matters is whether the function is a builtin function. We have the same miscompile if we removed all the `builtin` / `nobuiltin` annotations from the IR generated for the PR49022 example, and this change does nothing to address that case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97735