[PATCH] D4276: Added llvm.is.constant intrinsic

[James Y Knight via Phabricator via llvm-commits]

jyknight added a comment.

In https://reviews.llvm.org/D4276#1067497, @efriedma wrote:

> It seems like you're waiting to fold llvm.is.constant until really late in the optimization pipeline; we probably want to fold it sometime in the "middle", so we get better optimization. Maybe just after function simplification passes; at that point, we're unlikely to get any more useful information about whether the argument is constant, and we want to simplify the code as much as possible before we run transforms like loop vectorization.


The most important constraint is that it needs to occur after inlining.

However, we already have llvm.objectsize intrinsic, and I wanted to handle the final failure in the same place, so that __builtin_constant_p works when given __builtin_object_size as an argument, even when the __builtin_object_size is unevaluable (and returns -1).

E.g. in GCC, this code:

  int f1(char* x) {
      if (__builtin_constant_p(__builtin_object_size(x, 0))) {
          return __builtin_object_size(x, 0);
      }
      return -9999;
  }
  
  int f2() {
      char x[1000];
      return f1(x);
  }

compiles such that:

  f1 -> returns (constant) -1.
  f2 -> returns (constant) 1000.

It's possible that both of them should move somewhere else, though...



================
Comment at: llvm/lib/Analysis/ConstantFolding.cpp:1592
+    return true;
+  } else if (isa<ConstantAggregate>(c) || isa<ConstantExpr>(c)) {
+    for (const Value *subc : c->operand_values()) {
----------------
efriedma wrote:
> Probably doesn't make sense to check for ConstantExpr here; if isManifestConstant is true for all the operands of a ConstantExpr, it should get constant-folded.
If I remove that, then this clause fails in the test case:
  @llvm.is.constant.p0i64(i64* inttoptr (i32 42 to i64*))
because there's no way to express a pointer with a constant value directly.


https://reviews.llvm.org/D4276