Hoisting convergent function calls


Is it a bug for optimization passes to hoist calls to convergent functions out of control flow? If I run SimplifyCFG on test/Transforms/GVNHoist/hoist-convergent.ll it will do this and EarlyCSE does it as well when run on the following LLVM IR:

define float @no_convergent_func_hoisting(float %d, float %min, float %max, float %a) {


%div = fdiv float 1.000000e+00, %d

%cmp = fcmp oge float %div, 0.000000e+00

%sub1 = fsub float %max, %a

%c = call float @convergent_func(float %sub1, float %div)

br i1 %cmp, label %if.then, label %if.else


%mul2 = call float @convergent_func(float %sub1, float %div)

br label %if.end


%mul6 = call float @convergent_func(float %sub1, float %div)

br label %if.end


%tmax.0 = phi float [ %mul2, %if.then ], [ %mul6, %if.else ]

%add = fadd float %tmax.0, %c

ret float %add


declare float @convergent_func(float, float) #0

attributes #0 = { nounwind readnone convergent }

The LLVM language reference says this behavior isn’t a bug since it only forbids making functions control-dependent on additional values, and this is removing a control dependence. However, the hoist-convergent test seems to indicate that convergent functions should not be hoisted, and this is necessary for implementing cross-invocation operations like OpenGL’s ballotARB() since it could affect which invocations are active when the function is called.




Yes, convergent function calls are allowed to be hoisted. They're
supposed to model operations like derivatives and readInvocation()
that give the same result as long as the set of active invocations is
only ever enlarged. For ballotARB() and friends which can't be
hoisted, you need to add something else. RadeonSI, which supports the
ARB_shader_ballot GL extension with ballotARB(), adds a side-effecting
inline asm instruction beforehand to keep LLVM from hoisting the call:
but that's obviously pretty bad, and won't work at all if the function
calling ballotARB() isn't the entrypoint. Maybe just removing
IsSpeculatable will work? Athough I don't know if I fully understand
that attribute to be honest.

The test you referenced seems wrong to me. At the very least, whether
the function can be hoisted has nothing to do with whether it's
convergent or not. Seems like Matt added it a long time ago. maybe he
can explain if there's some subtlety I missed.