Fangqing Du via llvm-dev
2020-Jul-14 16:28 UTC
[llvm-dev] Bug in pass 'ipsccp' on function attribute 'argmemonly'?
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>
Alexandre Isoard via llvm-dev
2020-Jul-14 21:28 UTC
[llvm-dev] Bug in pass 'ipsccp' on function attribute 'argmemonly'?
I think here, I would fix ipsccp. It transform an access of a function argument based on that argument into an access of a function argument not based on that argument. While one could argue it is still argmemonly (the documentation is ambiguous on that) it makes it significantly harder to trace that correlation for deadargelim or any form of alias analysis. In general I wonder if we would not benefit from an attribute that list the globals that a function may access, and having argmemonly include those globals in what it consider "arg". On Tue, Jul 14, 2020, 09:29 Fangqing Du via llvm-dev < llvm-dev at lists.llvm.org> wrote:> 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 >> >> _______________________________________________ > 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/20200714/b7984b32/attachment.html>
Hal Finkel via llvm-dev
2020-Jul-14 21:34 UTC
[llvm-dev] Bug in pass 'ipsccp' on function attribute 'argmemonly'?
On 7/14/20 11:28 AM, Fangqing Du wrote:> Thank you Hal and Stefan! > > Bug report is filed: https://bugs.llvm.org/show_bug.cgi?id=46717 > <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?Correct. Each pass must be correct on its own. We need to fix ipsccp. -Hal> > Thanks, > Fangqing > > On Sat, Jul 11, 2020 at 5:12 AM Hal Finkel <hfinkel at anl.gov > <mailto: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/ <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 <mailto:llvm-dev at lists.llvm.org> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev> > > -- > Hal Finkel > Lead, Compiler Technology and Programming Languages > Leadership Computing Facility > Argonne National Laboratory >-- 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/2f453cc3/attachment.html>
Johannes Doerfert via llvm-dev
2020-Jul-15 00:00 UTC
[llvm-dev] Bug in pass 'ipsccp' on function attribute 'argmemonly'?
On 7/14/20 4:34 PM, Hal Finkel via llvm-dev wrote:> > On 7/14/20 11:28 AM, Fangqing Du wrote: >> Thank you Hal and Stefan! >> >> Bug report is filed: https://bugs.llvm.org/show_bug.cgi?id=46717 >> <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? > > Correct. Each pass must be correct on its own. We need to fix ipsccp. >I don't think that was ever questioned ;) I think the comment about the Attributor was more to show that this behavior is not the same there which is yet another good indicator this is a bug. ~ Johannes> -Hal > > >> >> Thanks, >> Fangqing >> >> On Sat, Jul 11, 2020 at 5:12 AM Hal Finkel <hfinkel at anl.gov >> <mailto: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/ <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 <mailto:llvm-dev at lists.llvm.org> >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>> <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev> >> >> -- Hal Finkel >> Lead, Compiler Technology and Programming Languages >> Leadership Computing Facility >> Argonne National Laboratory >> >> >> _______________________________________________ >> 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/20200714/fd2f1356/attachment-0001.html>