This is an archive of the discontinued LLVM Phabricator instance.

[LangRef] Clarify codegen expectations for intrinsics with fp/integer-only overloads
ClosedPublic

Authored by aemerson on Mar 21 2019, 11:26 AM.

Details

Summary

[LangRef] Clarify codegen expectations for intrinsics with fp/integer-only overloads.

This change is a result of discussions on list: "GlobalISel: Ambiguous intrinsic semantics problem"

Diff Detail

Event Timeline

aemerson created this revision.Mar 21 2019, 11:26 AM
arsenm added inline comments.Mar 21 2019, 11:41 AM
llvm/docs/LangRef.rst
10397–10401

I feel like this could be rephrased more generally as something along the lines of there shouldn't be a major semantic change in the operation based on the overloaded type. The integer/FP distinction is one particular example. I'm trying to come up with a definition for "major semantic change" means though.

I have a sort of counterexample though in the AMDGPU buffer intrinsics. If the return type is a struct, it changes the instruction slightly, but not in a problematic/ambiguous way like the FP/integer type.

I don't understand what this is supposed to mean to who the audience of this statement is?

aemerson marked an inline comment as done.Mar 21 2019, 12:19 PM

I don't understand what this is supposed to mean to who the audience of this statement is?

Clearly the wording isn't good enough then, but here's the motivating discussion: http://lists.llvm.org/pipermail/llvm-dev/2019-March/130903.html

The tl;dr is that we agreed that intrinsics which need different codegen based *only* on whether a intrinsic operand type is integer or fp is not supported.

call @llvm.foo(<4 x i32> %p) and call @llvm.foo(<4 x float> %p) should not be supported by the code generator, as the only thing that distinguishes them is the fp/integer type. With llvm::Instruction we have separate opcodes for fp/int operations, e.g. fadd vs add.

The audience of this are people who are defining new target intrinsics.

llvm/docs/LangRef.rst
10397–10401

Perhaps, but I think we should try to be specific.

The audience of this are people who are defining new target intrinsics.

This should be made explicitly clear. Thanks.

aemerson updated this revision to Diff 191784.Mar 21 2019, 3:00 PM

Re-wording and making it more explicit who this is for.

arsenm accepted this revision.Jun 24 2019, 3:11 PM

LGTM

This revision is now accepted and ready to land.Jun 24 2019, 3:11 PM
This revision was automatically updated to reflect the committed changes.