[PATCH] D75799: [JumpThreading] Don't create PHI nodes with "is.constant" values

[Kazu Hirata via Phabricator via llvm-commits]

kazu added a comment.

It looks like the testcase can be reduced to the following.  You can check for no jump threading occurring:

  define void @foo(i32 %a, i32 %var) {
  entry:
    %cond1 = icmp ugt i32 %a, 24
    br i1 %cond1, label %bb_constant, label %bb_cond
  
  bb_constant:
    br label %bb_cond
  
  bb_cond:
    %phi = phi i32 [ 24, %bb_constant ], [ %var, %entry ]
    %sext = sext i32 %phi to i64
    %cond2 = icmp ugt i64 %sext, 24
    %is_constant = call i1 @llvm.is.constant.i64(i64 %sext)
    br i1 %cond2, label %bb_then, label %bb_else
  
  bb_then:
    unreachable
  
  bb_else:
    unreachable
  }
  
  ; Function Attrs: nounwind readnone willreturn
  declare i1 @llvm.is.constant.i64(i64) #0
  
  attributes #0 = { nounwind readnone willreturn }

That said, if I remove the sign extension like so, the edge `entry->bb_constant` gets threaded through `bb_cond` to `bb_else` even with your patch.  IIUC, your patch does not catch a case where a PHI node is directly used by `llvm.is_constant`:

  define void @bar(i32 %a, i64 %var) {
  entry:
    %cond1 = icmp ugt i32 %a, 24
    br i1 %cond1, label %bb_constant, label %bb_cond
  
  bb_constant:
    br label %bb_cond
  
  bb_cond:
    %sext = phi i64 [ 24, %bb_constant ], [ %var, %entry ]
    %cond2 = icmp ugt i64 %sext, 24
    %is_constant = call i1 @llvm.is.constant.i64(i64 %sext)
    br i1 %cond2, label %bb_then, label %bb_else
  
  bb_then:
    unreachable
  
  bb_else:
    unreachable
  }
  
  ; Function Attrs: nounwind readnone willreturn
  declare i1 @llvm.is.constant.i64(i64) #0
  
  attributes #0 = { nounwind readnone willreturn }

By the way, `MaybeThreadThroughTwoBasicBlocks` in `JumpThreading.cpp` can thread an edge through two successive basic blocks theses days.  You need to teach `MaybeThreadThroughTwoBasicBlocks` to look out for problematic `llvm.is_constant` also.

FWIW, we catch convergent calls in a somewhat sneaky interface. Specifically, `getJumpThreadDuplicationCost` returns `~0U` when it encounters an instruction that we cannot safely duplicate.  You could teach `getJumpThreadDuplicationCost` to return `~0U` on a problematic `llvm.is_constant`.  This way, you don't have to perform the same check in both `MaybeThreadThroughTwoBasicBlocks` and `TryThreadEdge`.

Now, the `convergent` attribute seems to me like a perfect fit for `llvm.is_constant` as far as I can tell from its description, but I am not sure what side effects it would have on the rest of the compiler.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75799