Page MenuHomePhabricator

[SLC] Allow llvm.pow(2**n,x) -> llvm.exp2(n*x) even if no exp2 lib func
AbandonedPublic

Authored by foad on Mon, May 4, 3:56 AM.

Details

Summary

replacePowWithExp generates a call either to the llvm.exp2 intrinsic or
to the exp2 library function. Allow it to use the intrinsic even if the
library function is not available.

Diff Detail

Event Timeline

foad created this revision.Mon, May 4, 3:56 AM
Herald added a project: Restricted Project. · View Herald TranscriptMon, May 4, 3:56 AM
lebedev.ri added inline comments.Mon, May 4, 4:27 AM
llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
1551–1553

Was this previously reachable?

foad marked an inline comment as done.Mon, May 4, 5:57 AM
foad added inline comments.
llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
1551–1553

Yes whenever we try to optimize a call to the llvm.pow intrinsic on a target that also has the pow libfunc available.

arsenm added a subscriber: arsenm.Mon, May 4, 6:45 AM
arsenm added inline comments.
llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
1537–1538

I've always hated the way all of this code conflates library functions and intrinsics as if they're interchangeable.

When we're compiling C code, we can't blindly generate floating-point intrinsics. Even if we're confident the transform is correct, the backend has limited capabilities. If the corresponding libfunc doesn't exist, and the target doesn't have any special capabilities, the backed is stuck: we need to generate a function call, but the function doesn't exist.

We could potentially add some TTI query to ask about the intrinsic, specifically, as opposed to the library function. But we can't just bypass the check.

evandro accepted this revision.Mon, May 4, 3:09 PM

LGTM

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
1551

This can probably be cached into a variable at the top of the function.

This revision is now accepted and ready to land.Mon, May 4, 3:09 PM
efriedma requested changes to this revision.Mon, May 4, 3:44 PM
This revision now requires changes to proceed.Mon, May 4, 3:44 PM
foad marked 2 inline comments as done.Tue, May 5, 12:41 AM

When we're compiling C code, we can't blindly generate floating-point intrinsics. Even if we're confident the transform is correct, the backend has limited capabilities. If the corresponding libfunc doesn't exist, and the target doesn't have any special capabilities, the backed is stuck: we need to generate a function call, but the function doesn't exist.

We could potentially add some TTI query to ask about the intrinsic, specifically, as opposed to the library function. But we can't just bypass the check.

For background, I'm interested in the AMDGPU target which has no libfuncs at all, but certainly does support intrinsics like llvm.sin/cos/exp/log.f32 because there are hardware instructions for them.

My understanding was that generic (llvm.*) intrinsics were unconditionally supported by all targets, because they're part of the IR almost as much as Instructions are, and it's CodeGen's job to implement them one way or another.

But you're saying that this is not true, and furthermore there is no good way to query whether a particular intrinsic is supported by a particular target? Is there any way forward, short of implementing a new TTI query mechanism like you suggested?

The "intrinsic" part of an intrinsic just means that the call has special semantics. Some intrinsics can't be implemented by certain targets, or don't make sense. And some intrinsics which make sense on a target in theory might not actually be implemented. This is actually true for instructions as well, although the cases where this comes up are more rare.

Fundamentally, the issue is that when compiling C, LLVM has limited control over the link and runtime environment. If an environment requires linking against a libc that doesn't have exp2, we don't have any control over that, and can't really do anything about it, unless we want to teach the compiler to emit an implementation inline.

We currently don't have any uniform way of dealing with this at the moment; if you're interested in adding one, it would make sense.

foad abandoned this revision.Tue, May 5, 2:18 PM