Abandoning. I agree with relying on attribute speculatable. If we figure out the conditions that make a convergent call speculatable, then at that time, we should use the appropriate attribute to record that fact. On the other hand, if speculation of a convergent call is dependent on the surrounding control flow, then we will probably have to do it as a separate optimization.

LGTM, with a few nits.

Bump! Waiting for more info on comments from @arsenm

Superseded by D152431.

Superseded by D147116

• rebased
• addressed some review comments
• added machine verifier

• updated the lit test to match the changed error message

@AlexM FYI: submitted a different fix in the same location: 4d081560cd3b6ac114aee15f50480dd978b55a44

• rebased
• fixed formatting near assert
• added clang-format off/on to allow existing code through pre-checks

Can you please reword the change description? Rather than talking about the code in terms of if/while, it's more helpful to talk about what is happening. For example, "we should first check for an irreducible outermost cycle before checking if the parent is reducible".

• rebased
• machine verifier disallows NoConvergent on opcodes that are not isConvergent
• new test for the machine verifier
• added missing comment

In D157475#4574293, @arsenm wrote:

I have a mostly updated copy of the noconvergent patch, fixing the remaining tests seems permanently stuck as the #3 thing on my todo list. I've also been debating splitting the existing isConvergent MI property into two halves (doesn't really matter for this patch, it's a lot easier to change MI flags than IR ones)

Adding @xbolva00, @RKSimon and @craig.topper since a lot of thought seems to have been involved when the size of MIFlags was bumped to uint32_t, in D118118. Note that we intend to use this new flag MIFlags::NoConvergent only in GlobalISel.

In D157389#4569735, @arsenm wrote:

In D157389#4569730, @sameerds wrote:

What is the "old syncscope parameter" mentioned in the change description? In the actual change and the lit tests, the default "system" scope is being changed to "agent", which superficially looks like an incorrect change.

In the deleted intrinsic. There was an integer syncscope argument which was never actually used. This just needs to try emulating the old intrinsic behavior

What is the "old syncscope parameter" mentioned in the change description? In the actual change and the lit tests, the default "system" scope is being changed to "agent", which superficially looks like an incorrect change.

• rebase
• fix build failures:
  • Shared libraries build failed with undefined references due to "extern template" declarations.
  • Modules build failed due to a cycle dependence between llvm/ADT and llvm/IR. The Generic*Impl.h files should be in llvm/IR to prevent this.

Reopened this because of build errors with shared libraries enabled.

In D156522#4549644, @c-rhodes wrote:

this broke my build with:

ld.lld-12: error: undefined symbol: llvm::GenericCycle<llvm::GenericSSAContext<llvm::Function> >::contains(llvm::BasicBlock const*) const
>>> referenced by UniformityAnalysis.cpp:72 (/home/culrho01/llvm-project/llvm/lib/Analysis/UniformityAnalysis.cpp:72)
>>>               lib/Analysis/CMakeFiles/LLVMAnalysis.dir/UniformityAnalysis.cpp.o:(llvm::GenericUniformityAnalysisImpl<llvm::GenericSSAContext<llvm::Function> >::usesValueFromCycle(llvm::Instruction const&, llvm::GenericCycle<llvm::GenericSSAContext<llvm::Function> > const&) const)

please could you look into it?

• rebase
• replaced std::function with function_ref
• removed the everything capture
• renamed CheckFailed() to reportFailure() ... more aligned with the coding standard

• rebased
• replaced the "Explicit" functions with overloads
• s/Code/Opcode/
• renamed verifyIntrinsic* functions

• fixed missing headers

• reverted and fixed buildbot breakage
• there were dangling uses of MachineInstr::getIntrinsicID() in other targets
• note to self: don't forget to check Phab for a successful build before submitting!

• rebased
• added the GIntrinsic::is() method
• fixed issues with coding style

Bump! This is blocking further changes. Added some more reviewers to get more eyeballs.