[PATCH] D27855: [InstCombine] try to extend nonnull-ness of arguments from a callsite back to its parent function

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In https://reviews.llvm.org/D27855#626165, @silvas wrote:

> In https://reviews.llvm.org/D27855#626141, @spatel wrote:
>
> > This would be a good argument for Mehdi's suggestion? Ie, this should be in FunctionAttrs.cpp?
>
>
> Yes. I think that's the natural place to put this.


I have a draft of a patch to put this in FunctionAttrs instead of InstCombine which looks straightforward, but there's a problem:

  define i1 @bar(i32* nonnull %x) {
    %ld = load i32, i32* %x
    %cmp = icmp eq i32 %ld, 42
    ret i1 %cmp
  }
  
  define i1 @foo(i32* %x) { ; we want this param marked 'nonnull'
    %d = call i1 @bar(i32* %x)
    %null_check = icmp eq i32* %x, null ; foo() always returns the result of bar()
    br i1 %null_check, label %t, label %f
  t:
    ret i1 1
  f:
    ret i1 %d
  }

$ ./opt -functionattrs -inline -instcombine -simplifycfg isa12.ll -S
...

  define i1 @foo(i32* nonnull readonly %x) #0 {
  f:
    %ld.i = load i32, i32* %x, align 4
    %cmp.i = icmp eq i32 %ld.i, 42 ; success! no null check
    ret i1 %cmp.i
  }

$ ./opt -O2 isa12.ll -S
...

  define i1 @foo(i32* readonly %x) local_unnamed_addr #0 {
  t:
    %ld.i = load i32, i32* %x, align 4
    %cmp.i = icmp eq i32 %ld.i, 42
    %null_check = icmp eq i32* %x, null ; FAIL
    %.d = or i1 %null_check, %cmp.i
    ret i1 %.d
  }

$ ./opt -O2 -debug-pass=Arguments -S < /dev/null 
...
Pass Arguments:  
...
-inline -functionattrs

In the -O2 pipeline, -functionattrs is only called once (directly after -inline), so we hit the exact problem that I'm trying to avoid: the information provided by the callsite's arg attribute is gone. Is a change to the -O2 pipeline an acceptable follow-up to this patch?


https://reviews.llvm.org/D27855