llvm dev - Jul 2020 - Bug in pass 'ipsccp' on function attribute 'argmemonly'?

Hi all,

Let's see an example (from Alexandre Isoard) first:
****************************************************************************************
; RUN: opt -ipsccp -deadargelim -licm -O2 -S < %s

@g = internal global i32 0

; Function Attrs: argmemonly
define internal void @foo(i32* nonnull dereferenceable(4) %arg, i32 %val) #0
 {
entry:
  store i32 %val, i32* %arg
  ret void
}

define i32 @bar(i32 %n) {
entry:
  store i32 1, i32* @g
  br label %loop

loop:                                              ; preds = %bb1, %bb
  %i = phi i32 [ %i.inc, %loop ], [ 0, %entry ]
  %g.val = load i32, i32* @g
  %g.inc = add nuw i32 %g.val, 1
  tail call void @foo(i32* @g, i32 %g.inc)
  %i.inc = add nuw i32 %i, 1
  %cond = icmp eq i32 %i.inc, %n
  br i1 %cond, label %exit, label %loop

exit:                                              ; preds = %bb1
  ret i32 %g.val
}

declare void @llvm.assume(i1)

attributes #0 = { argmemonly }
****************************************************************************************
With opt cmd '-ipsccp -deadargelim -licm -O2', function @bar will return
constant value 1, instead of correct value %n. This is crazy, right?
Let's see what happens here.
Due to pass 'ipsccp' replaced the argument of function '@foo'
with global
variable '@g', but keeps attr 'argmemonly' there, so pass
'licm' will hoist
the load of '@g' before the loop (as the value of @g isn't changed,
but it
is changed), and will cause function return wrong constant value '1',
instead of '%n' (the correct value) , like following:
****************************************************************************************

; Function Attrs: nofree norecurse nounwind writeonly
define i32 @bar(i32 %n) local_unnamed_addr #0 {
entry:
  ret i32 1
}

attributes #0 = { nofree norecurse nounwind writeonly }
****************************************************************************************


And if remove the 'argmemonly' attribute, function @bar will return the
correct value:
****************************************************************************************

; Function Attrs: nofree norecurse nounwind
define i32 @bar(i32 %n) local_unnamed_addr #0 {
entry:
  ret i32 %n
}
****************************************************************************************


So if function attribute 'argmemonly' on function @foo is correct, then
there's a bug in pass 'ipsccp'. When it replaces the function local
value
with global variable, then it shoud remember to remove this function
attribute.

Thanks,
Fangqing
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200710/71e12c6e/attachment.html>

Hi Fangqing,

It looks like that after -deadargelim `foo()` should not have an
`argmemonly` attribute. If you also run Attributor here, you get the
expected result: https://godbolt.org/z/8hcG9c

Unfortunately, Attributor doesn't yet run by default.

-Stefan

On Sat, Jul 11, 2020 at 4:05 AM Fangqing Du via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> Hi all,
>
> Let's see an example (from Alexandre Isoard) first:
>
>
****************************************************************************************
> ; RUN: opt -ipsccp -deadargelim -licm -O2 -S < %s
>
> @g = internal global i32 0
>
> ; Function Attrs: argmemonly
> define internal void @foo(i32* nonnull dereferenceable(4) %arg, i32 %val
> ) #0 {
> entry:
>   store i32 %val, i32* %arg
>   ret void
> }
>
> define i32 @bar(i32 %n) {
> entry:
>   store i32 1, i32* @g
>   br label %loop
>
> loop:                                              ; preds = %bb1, %bb
>   %i = phi i32 [ %i.inc, %loop ], [ 0, %entry ]
>   %g.val = load i32, i32* @g
>   %g.inc = add nuw i32 %g.val, 1
>   tail call void @foo(i32* @g, i32 %g.inc)
>   %i.inc = add nuw i32 %i, 1
>   %cond = icmp eq i32 %i.inc, %n
>   br i1 %cond, label %exit, label %loop
>
> exit:                                              ; preds = %bb1
>   ret i32 %g.val
> }
>
> declare void @llvm.assume(i1)
>
> attributes #0 = { argmemonly }
>
>
****************************************************************************************
> With opt cmd '-ipsccp -deadargelim -licm -O2', function @bar will
return
> constant value 1, instead of correct value %n. This is crazy, right?
> Let's see what happens here.
> Due to pass 'ipsccp' replaced the argument of function
'@foo' with global
> variable '@g', but keeps attr 'argmemonly' there, so pass
'licm' will hoist
> the load of '@g' before the loop (as the value of @g isn't
changed, but it
> is changed), and will cause function return wrong constant value
'1',
> instead of '%n' (the correct value) , like following:
>
****************************************************************************************
>
> ; Function Attrs: nofree norecurse nounwind writeonly
> define i32 @bar(i32 %n) local_unnamed_addr #0 {
> entry:
>   ret i32 1
> }
>
> attributes #0 = { nofree norecurse nounwind writeonly }
>
****************************************************************************************
>
>
> And if remove the 'argmemonly' attribute, function @bar will return
the
> correct value:
>
****************************************************************************************
>
> ; Function Attrs: nofree norecurse nounwind
> define i32 @bar(i32 %n) local_unnamed_addr #0 {
> entry:
>   ret i32 %n
> }
>
****************************************************************************************
>
>
> So if function attribute 'argmemonly' on function @foo is correct,
then
> there's a bug in pass 'ipsccp'. When it replaces the function
local value
> with global variable, then it shoud remember to remove this function
> attribute.
>
> Thanks,
> Fangqing
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200711/675015aa/attachment.html>

Hi, Fangqing,

Yes, this seems like a bug. Can you please file a bug report at 
https://bugs.llvm.org/ ? If you need someone else to do this on your 
behalf, please let us know.

  -Hal

On 7/10/20 9:04 PM, Fangqing Du via llvm-dev wrote:
> Hi all,
>
> Let's see an example (from Alexandre Isoard) first:
>
****************************************************************************************
> ; RUN: opt -ipsccp -deadargelim -licm -O2 -S < %s
>
> @g = internal global i32 0
>
> ; Function Attrs: argmemonly
> define internal void @foo(i32* nonnull dereferenceable(4) %arg, i32 
> %val) #0 {
> entry:
>   store i32 %val, i32* %arg
>   ret void
> }
>
> define i32 @bar(i32 %n) {
> entry:
>   store i32 1, i32* @g
>   br label %loop
>
> loop: ; preds = %bb1, %bb
> %i = phi i32 [ %i.inc, %loop ], [ 0, %entry ]
> %g.val = load i32, i32* @g
> %g.inc = add nuw i32 %g.val, 1
> tail call void @foo(i32* @g, i32 %g.inc)
> %i.inc = add nuw i32 %i, 1
> %cond = icmp eq i32 %i.inc, %n
>   br i1 %cond, label %exit, label %loop
>
> exit: ; preds = %bb1
>   ret i32 %g.val
> }
>
> declare void @llvm.assume(i1)
>
> attributes #0 = { argmemonly }
>
****************************************************************************************
>
> With opt cmd '-ipsccp -deadargelim -licm -O2', function @bar will 
> return constant value 1, instead of correct value %n. This is crazy, 
> right?
> Let's see what happens here.
> Due to pass 'ipsccp' replaced the argument of function
'@foo' with
> global variable '@g', but keeps attr 'argmemonly' there, so
pass
> 'licm' will hoist the load of '@g' before the loop (as the
value of @g
> isn't changed, but it is changed), and will cause function return 
> wrong constant value '1', instead of '%n' (the correct
value) , like
> following:
>
****************************************************************************************
>
> ; Function Attrs: nofree norecurse nounwind writeonly
> define i32 @bar(i32 %n) local_unnamed_addr #0 {
> entry:
> ret i32 1
> }
>
> attributes #0 = { nofree norecurse nounwind writeonly }
>
****************************************************************************************
>
>
> And if remove the 'argmemonly' attribute, function @bar will return
> the correct value:
>
****************************************************************************************
>
> ; Function Attrs: nofree norecurse nounwind
> define i32 @bar(i32 %n) local_unnamed_addr #0 {
> entry:
> ret i32 %n
> }
>
****************************************************************************************
>
>
> So if function attribute 'argmemonly' on function @foo is correct, 
> then there's a bug in pass 'ipsccp'. When it replaces the
function
> local value with global variable, then it shoud remember to remove 
> this function attribute.
>
> Thanks,
> Fangqing
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200711/475835f3/attachment.html>

Thank you Hal and  Stefan!

Bug report is filed: https://bugs.llvm.org/show_bug.cgi?id=46717

And Stefan,
I think 'attributor' is a really nice pass, and can infer more precise
and
useful attributes, which may give more opportunities for optimization.
But we shouldn't depend on 'attributor' to correct wrong function
attributes, because we cannot run 'attributor' after every pass, right?

Thanks,
Fangqing

On Sat, Jul 11, 2020 at 5:12 AM Hal Finkel <hfinkel at anl.gov> wrote:
> Hi, Fangqing,
>
> Yes, this seems like a bug. Can you please file a bug report at
> https://bugs.llvm.org/ ? If you need someone else to do this on your
> behalf, please let us know.
>
>  -Hal
> On 7/10/20 9:04 PM, Fangqing Du via llvm-dev wrote:
>
> Hi all,
>
> Let's see an example (from Alexandre Isoard) first:
>
>
****************************************************************************************
> ; RUN: opt -ipsccp -deadargelim -licm -O2 -S < %s
>
> @g = internal global i32 0
>
> ; Function Attrs: argmemonly
> define internal void @foo(i32* nonnull dereferenceable(4) %arg, i32 %val
> ) #0 {
> entry:
>   store i32 %val, i32* %arg
>   ret void
> }
>
> define i32 @bar(i32 %n) {
> entry:
>   store i32 1, i32* @g
>   br label %loop
>
> loop:                                              ; preds = %bb1, %bb
>   %i = phi i32 [ %i.inc, %loop ], [ 0, %entry ]
>   %g.val = load i32, i32* @g
>   %g.inc = add nuw i32 %g.val, 1
>   tail call void @foo(i32* @g, i32 %g.inc)
>   %i.inc = add nuw i32 %i, 1
>   %cond = icmp eq i32 %i.inc, %n
>   br i1 %cond, label %exit, label %loop
>
> exit:                                              ; preds = %bb1
>   ret i32 %g.val
> }
>
> declare void @llvm.assume(i1)
>
> attributes #0 = { argmemonly }
>
>
****************************************************************************************
> With opt cmd '-ipsccp -deadargelim -licm -O2', function @bar will
return
> constant value 1, instead of correct value %n. This is crazy, right?
> Let's see what happens here.
> Due to pass 'ipsccp' replaced the argument of function
'@foo' with global
> variable '@g', but keeps attr 'argmemonly' there, so pass
'licm' will hoist
> the load of '@g' before the loop (as the value of @g isn't
changed, but it
> is changed), and will cause function return wrong constant value
'1',
> instead of '%n' (the correct value) , like following:
>
****************************************************************************************
>
> ; Function Attrs: nofree norecurse nounwind writeonly
> define i32 @bar(i32 %n) local_unnamed_addr #0 {
> entry:
>   ret i32 1
> }
>
> attributes #0 = { nofree norecurse nounwind writeonly }
>
****************************************************************************************
>
>
> And if remove the 'argmemonly' attribute, function @bar will return
the
> correct value:
>
****************************************************************************************
>
> ; Function Attrs: nofree norecurse nounwind
> define i32 @bar(i32 %n) local_unnamed_addr #0 {
> entry:
>   ret i32 %n
> }
>
****************************************************************************************
>
>
> So if function attribute 'argmemonly' on function @foo is correct,
then
> there's a bug in pass 'ipsccp'. When it replaces the function
local value
> with global variable, then it shoud remember to remove this function
> attribute.
>
> Thanks,
> Fangqing
>
> _______________________________________________
> LLVM Developers mailing listllvm-dev at
lists.llvm.orghttps://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
> --
> Hal Finkel
> Lead, Compiler Technology and Programming Languages
> Leadership Computing Facility
> Argonne National Laboratory
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200714/f04b88c2/attachment-0001.html>