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>
Alexandre Isoard via llvm-dev
2020-Jul-22 15:16 UTC
[llvm-dev] Bug in pass 'ipsccp' on function attribute 'argmemonly'?
Note that there is the same issue in ipconstprop (on the same example) for the same reason. (thanks @Lin-Ya Yu <yu810226 at gmail.com>) On Tue, Jul 14, 2020 at 5:02 PM Johannes Doerfert via llvm-dev < llvm-dev at lists.llvm.org> wrote:> > 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> > <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> <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/> > <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> > <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> > <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 listllvm-dev at lists.llvm.orghttps://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >-- *Alexandre Isoard* -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200722/5156c957/attachment.html>
Florian Hahn via llvm-dev
2020-Jul-23 17:03 UTC
[llvm-dev] Bug in pass 'ipsccp' on function attribute 'argmemonly'?
Thanks for the report. I put up a fix https://reviews.llvm.org/D84432 <https://reviews.llvm.org/D84432>> On Jul 22, 2020, at 16:16, Alexandre Isoard via llvm-dev <llvm-dev at lists.llvm.org> wrote: > > Note that there is the same issue in ipconstprop (on the same example) for the same reason. (thanks @Lin-Ya Yu <mailto:yu810226 at gmail.com>)I am not aware of anyone actively using the pass or working on it. If there are users, I’d love to hear, but otherwise I think we should consider removing it. It is a source of confusion and requires API updates from time to time. I put up a patch to remove it, with some more details:https://reviews.llvm.org/D84447 <https://reviews.llvm.org/D84447> -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200723/8c76e5a4/attachment.html>