[PATCH] D102367: [Intrinsics] is_constant intrinsic on undef evaluates to false

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D102367#2755705 <https://reviews.llvm.org/D102367#2755705>, @nickdesaulniers wrote:

>> While I think it would be correct to return i1 true when passing undef ..., it would also be correct to return i1 false
>
> I think it's better to return `i1 false`, otherwise we diverge from GCC on the test case I provided above:
>
>   int foo(void) {
>       int i;
>       return __builtin_constant_p(i);
>   }

You will not be able to get consistent semantics by special-casing undef. Yes, for your above example your new implementation will now return false, but if you do some trivial change like replacing `i` with `i & 1` it will return true again, because we fold `undef & 1` to zero. Undef can always be refined to a constant value.

> (and disagree with ourselves when optimizations are enabled)

This is not a problem per se, LangRef explicitly allows this: "The result also intentionally depends on the result of optimization passes".



================
Comment at: llvm/lib/Analysis/ConstantFolding.cpp:1809
 
 static bool isManifestConstant(const Constant *c) {
   if (isa<ConstantData>(c)) {
----------------
Maybe you want to export this helper (or add it to Constant) and use it in LowerConstantIntrinsics? Seems like these should be using the same logic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102367