willreturn

After the '[local] treat calls that may not return as being alive' patch, https://reviews.llvm.org/D94106

it seems not to be possible in clang to indicate that a 'C' function or an 'extern "C"' function will progress.
At least, I did not find an attribute that indicates this. ( __attribute__((const)) is not sufficient)

Is there already a clang attribute to indicate this ?

Thanks,

Jeroen Dobbelaere

I'm confused.

Are you interested in adding `willreturn` to a function via clang?

I'm confused.

Are you interested in adding `willreturn` to a function via clang?

I think so (also getting a little confused :frowning: ) ..
What should the behavior be of a '__attribute__((const))' function ?
Is the progress guaranteed ?

See Compiler Explorer

// 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'.

Greetings,

Jeroen Dobbelaere

I'm confused.

Are you interested in adding `willreturn` to a function via clang?

I think so (also getting a little confused :frowning: ) ..
What should the behavior be of a '__attribute__((const))' function ?
Is the progress guaranteed ?

See Compiler Explorer

// 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.

This is "just" a phase ordering issue. Run O3 again and trunk happily removes it: Compiler Explorer
That said, I agree, we should check why this is happening and what to do about it.

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.

~ Johannes

Hi,

I’m confused.

Are you interested in adding willreturn to a function via clang?

I think so (also getting a little confused :frowning: ) …
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
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

Hi,

I'm confused.

Are you interested in adding `willreturn` to a function via clang?

I think so (also getting a little confused :frowning: ) ..
What should the behavior be of a '__attribute__((const))' function ?
Is the progress guaranteed ?

See Compiler Explorer

// 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: Compiler Explorer
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 :wink:

Sounds good.

I also put up a patch to add this logic to FunctionAttrs, so we have something we can easily pick for the 12.x release: https://reviews.llvm.org/D96949

Cheers,
Florian

And I found out that, according to the gcc description, attribute((pure)) and attribute((const)) functions must always return.

I put up a patch that adds the ‘willreturn’ attribute: https://reviews.llvm.org/D96960

Greetings,

Jeroen Dobbelaere