[llvm-dev] [RFC] Introducing the maynotprogress IR attribute

[Atmn Patel via llvm-dev]

Hi All,

Based on the discussion, we changed the implementation so that there is a
`mustprogress` function attribute that is applied to all C++ functions that
do not contain a loop with a non-zero constant conditional, and a loop
metadata `llvm.loop.mustprogress` that is applied to each loop without a
constant conditional when compiled with standards C11/C++11 or later. The
relevant patches are: D85393, D86233, D87262, D88464, D86841, and D86844,
some of which have already been accepted, and are ready to be upstreamed,
and I will start doing so shortly pending no additional design comments.
There will be follow-up patches for fixing more miscompilations that are
currently done by the existing passes.

Atmn

On Fri, Sep 11, 2020 at 3:55 PM James Y Knight <jyknight at google.com> wrote:

>
>
> On Fri, Sep 11, 2020 at 2:33 PM Atmn Patel <atmndp at gmail.com> wrote:
>
>> Hi James,
>>
>> On Fri, Sep 11, 2020 at 2:05 PM James Y Knight <jyknight at google.com>
>> wrote:
>> >
>> > On Fri, Sep 11, 2020 at 12:42 PM Atmn Patel <atmndp at gmail.com> wrote:
>> >>
>> >> Hi Hal,
>> >>
>> >> On Thu, Sep 10, 2020 at 8:54 PM Hal Finkel <hfinkel at anl.gov> wrote:
>> >> >
>> >> > Hi, Atmn,
>> >> >
>> >> > Has anyone else expressed an opinion regarding the naming? We need to
>> >> > clarify the semantics in C, it seems.
>> >>
>> >> No other names have come in yet, in total the names proposed so far (I
>> >> think) are:
>> >> - maynotprogress
>> >> - maybenoprogress
>> >> - might_not_progress
>> >> - nfpg
>> >> - no_fpg
>> >> and the loop metadata has been pretty firmly established as
>> >> llvm.loop.mustprogress. IMHO, I've warmed up to no_fpg (or even nfpg
>> >> in a pinch if we want to save 33% of space and lose some readability)
>> >> since we've made substantial progress in clarifying the exact
>> >> definitions of progress in this context and I think it's a good idea
>> >> to bake it into the attribute name.
>> >
>> >
>> > I'd actually like to suggest that we invert the default for functions.
>> Rather than adding a "maynotprogress" function attribute, instead add a
>> "mustprogress" function attribute, which Clang will emit on every function
>> compiled in C++ mode. For two reasons:
>> > 1. Having both the attribute and the loop metadata be the same way
>> around makes it simpler to think about (rather than one being positive, and
>> the other being negated).
>> > 2. Given that the global progress-requirement seems to be pretty much
>> C++-specific, having this behavior be off by default, and opted into by C++
>> frontends makes sense.
>> > 3. Bonus: it makes choosing an attribute name easier: mustprogress,
>> done.
>> >
>> >> I've also modified the clang patch [0] to only apply either of the
>> >>
>> >> attributes for C functions when compiled with C11 or later so we can
>> >> tightly adhere to both the C and C++ standards, and the other changes
>> >> that need to be made will be forthcoming. Thanks again to James, that
>> >> particular example was pretty cool, and I agree that it may be best to
>> >> follow that interpretation.
>> >>
>> >> [0] https://reviews.llvm.org/D86841
>> >
>> >
>> > You mean that you now apply maynotprogress to all functions in C,
>> right? But why only C11 and later? I think all versions of C should get the
>> maynotprogress function attribute? (Or, with the change I suggest above:
>> only C++ code should get the "mustprogress" function attribute.)
>>
>> I actually couldn't find that particular statement in the C99 spec or
>> before, so all functions in C when compiled with C11 or later. So with
>> the changes you suggested, it'd be pre-C11 and C++ with the
>> "mustprogress".
>>
>
> If C99 doesn't have any statement about progress being required that
> doesn't imply that progress is always required. Rather the opposite -- that
> progress was not ever required until C11. So, we shouldn't add any
> mustprogress annotations in C89/99.
>
> It also looks as though the C++ text was only added in C++11, so we also
> shouldn't add any mustprogress annotations in C++98/03 modes.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20201013/481bcdc4/attachment.html>