Hi,> On Feb 17, 2021, at 19:15, Johannes Doerfert <johannesdoerfert at gmail.com> wrote: > > > On 2/17/21 12:04 PM, Jeroen Dobbelaere wrote: >>> I'm confused. >>> >>> Are you interested in adding `willreturn` to a function via clang? >> I think so (also getting a little confused :( ) .. >> What should the behavior be of a '__attribute__((const))' function ? >> Is the progress guaranteed ? >> >> See https://www.godbolt.org/z/GoPYhM >> >> >> // extern "C" ... >> extern int ptrfun1(int, int*, int*) __attribute__((const)); >> >> int b[5]; >> >> int foo() { >> int a[10]; >> int index = ptrfun1(5, &a[0], &b[0]); >> a[index]=10; >> return a[index]; >> } >> >> clang-11 is able to optimize this away >> >> clang-trunc is mixed: >> - for this case, the call will not optimize it away. (as far as I see, since D94106) >> - But if you do not use the return value upfront, it will be optimized away. >> ... >> int index = 3; >> ptrfun1(5, &a[0], &b[0]); >> ... >> >> If a '__attribute__((const))' function is allowed to not progress, then not all llvm passes are aware of this >> with the current mapping on llvm attributes and imho, we'll need an attribute to indicate that we want >> to have progress. Or, clang should map '__attribute__((const))' to 'readnone willreturn'. > > FWIW, I think removal is correct because you run it as C++ program. >Yep, I think that’s the case.> This is "just" a phase ordering issue. Run O3 again and trunk happily removes it: https://www.godbolt.org/z/95qeah <https://www.godbolt.org/z/95qeah> > That said, I agree, we should check why this is happening and what to do about it. >I think the reason this gets removed after another -O3 run is that there still are passes that may remove functions, even if they may not return. In this case it appears to be Bit-Tracking Dead Code Elimination.> Now wrt. the attribute lowering (const/pure) I think we could add willreturn iff the standard is C++11 or newer, but > not otherwise. For C++ before 11 and C we can always have infinite loops with constant conditions, IIRC.I think what we need to do is propagate information from function attributes to the call sites in the function. For mustprogress functions, all call sites should also be mustprogress I think. If the called function is also readnone (as in the const case), we should be able to add willreturn to the call site. So I think for the C++ case, all the needed information should already be available, we just need to make use of it. The question is mostly where we should do that? FunctionAttrs? On the C side of things, it might be useful to have an attribute to indicate that a function will always return. Cheers, Florian -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210217/953eb03d/attachment.html>
On 2/17/21 2:19 PM, Florian Hahn wrote:> Hi, > >> On Feb 17, 2021, at 19:15, Johannes Doerfert <johannesdoerfert at gmail.com> wrote: >> >> >> On 2/17/21 12:04 PM, Jeroen Dobbelaere wrote: >>>> I'm confused. >>>> >>>> Are you interested in adding `willreturn` to a function via clang? >>> I think so (also getting a little confused :( ) .. >>> What should the behavior be of a '__attribute__((const))' function ? >>> Is the progress guaranteed ? >>> >>> See https://www.godbolt.org/z/GoPYhM >>> >>> >>> // extern "C" ... >>> extern int ptrfun1(int, int*, int*) __attribute__((const)); >>> >>> int b[5]; >>> >>> int foo() { >>> int a[10]; >>> int index = ptrfun1(5, &a[0], &b[0]); >>> a[index]=10; >>> return a[index]; >>> } >>> >>> clang-11 is able to optimize this away >>> >>> clang-trunc is mixed: >>> - for this case, the call will not optimize it away. (as far as I see, since D94106) >>> - But if you do not use the return value upfront, it will be optimized away. >>> ... >>> int index = 3; >>> ptrfun1(5, &a[0], &b[0]); >>> ... >>> >>> If a '__attribute__((const))' function is allowed to not progress, then not all llvm passes are aware of this >>> with the current mapping on llvm attributes and imho, we'll need an attribute to indicate that we want >>> to have progress. Or, clang should map '__attribute__((const))' to 'readnone willreturn'. >> FWIW, I think removal is correct because you run it as C++ program. >> > Yep, I think that’s the case. > >> This is "just" a phase ordering issue. Run O3 again and trunk happily removes it: https://www.godbolt.org/z/95qeah <https://www.godbolt.org/z/95qeah> >> That said, I agree, we should check why this is happening and what to do about it. >> > I think the reason this gets removed after another -O3 run is that there still are passes that may remove functions, even if they may not return. In this case it appears to be Bit-Tracking Dead Code Elimination. > >> Now wrt. the attribute lowering (const/pure) I think we could add willreturn iff the standard is C++11 or newer, but >> not otherwise. For C++ before 11 and C we can always have infinite loops with constant conditions, IIRC. > I think what we need to do is propagate information from function attributes to the call sites in the function. For mustprogress functions, all call sites should also be mustprogress I think. If the called function is also readnone (as in the const case), we should be able to add willreturn to the call site. So I think for the C++ case, all the needed information should already be available, we just need to make use of it. The question is mostly where we should do that? FunctionAttrs?Attributor does that ;)> > On the C side of things, it might be useful to have an attribute to indicate that a function will always return. > > Cheers, > Florian