[PATCH] D90231: [GVN] Don't replace argument to @llvm.is.constant.*()

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa added reviewers: joerg, void.
jonpa added a comment.

> Was semantics of LLVM's @llvm.is.constant defined in some document since the last time @llvm.is.constant patch?

I see in the reference manual: "If its argument is known to be a manifest compile-time constant value, then the intrinsic will be converted to a constant true value. Otherwise, it will be converted to a constant false value..."  So clearly, having GVN turn this into a run-time check is wrong.

> This (especially the Local.cpp changes) seem rather heavy handed.

I am open to alternatives... I think it makes general sense to not optimize the argument to @llvm.is.constant. It might be possible to replace it as long as the new value falls into the same category as the old one of being compile-time constant or not, but that seems like unnecessary work to me. Unless it would help some later optimization somehow to produce better code?

This was my first idea of a handling to illustrate the problem, and it has been confirmed to resolve the problems in the original test case.

Since these methods by nature do not replace *all* uses, but only the dominated/non-local ones, it seems safe to me to exclude a user from the transformation. The caller is not expecting to always be able to remove the old value from the function.


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

https://reviews.llvm.org/D90231