[Attributor][FIX] Do not delete non`-mustprogress` calls

[jdoerfert]

Summary

Even if a call has no side-effects and cannot unwind, we need to
preserve it unless it is mustprogress, termination is a thing now.

Came up as part of D94633, test case included in mustprogress.ll

[nikic]

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

2888

Shouldn't this be checking willreturn rather than mustprogress?

[jdoerfert]

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

2888

Hm, willreturn is stronger, isn't it (it implies mustprogress IIRC)?
But here we also require no side-effects so they are probably equivalent.
What do you think?

[jdoerfert]

Add more mustprogress/willreturn recursion tests

[nikic]

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

2888

I think semantically, willreturn is the correct one here. mustprogress works because we assume that mustprogress + readonly = willreturn. However, it would be good to have that knowledge encoded only in one place (willreturn inference), rather than in multiple. Especially as it is not strictly speaking correct, if we wanted to be pedantic about atomic loads (we don't want to though :).

Generally, I think that the mustprogress attribute should only be placed by the frontend, and not derived. Only willreturn should be derived, and that's the property that analyses should be checking.

[jdoerfert]

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

2888

I think semantically, willreturn is the correct one here. mustprogress works because we assume that mustprogress + readonly = willreturn.

I think I agree. Will change it.

Generally, I think that the mustprogress attribute should only be placed by the frontend, and not derived.

Hm, maybe. It's not that the mustprogress deduction in D94740 actually does much but what it does cannot be replaced by willreturn, I think. Take

static void foo() { ... }
void bar() __attribute__((mustprogress)) { foo() }

We can conclude that foo needs to make progress but it might not return/unwind at all.
Maybe I'm confused again.

[jdoerfert]

Termination is only a side-effect if mustprogress is set, otherwise it is not.
I will rebase this and hope to land it soon after.