getMangledTypeStr: clarify how it mangles types, and add tests

[artagnon]

Summary

Write a set of tests that show how name mangling is done for overloaded intrinsics.

[sanjoy]

Two nits inline. Also, do we need a test case here?

lib/IR/Function.cpp

451 ↗

(On Diff #17979)

Modify this comment to reflect that we handle vectors too.

466 ↗

(On Diff #17979)

I think this will be better as VectorType* VTyp = dyn_cast<VectorType>(Ty)

[artagnon]

After some investigation, I have concluded that this change is
completely incorrect. For integers, floats and vectors, it is supposed
to fall back to the getEVTString() which may then return "i", "v",
"f" prefixes in the common case.

I have instead focused on updating the comment with this information,
and writing a set of tests that people can use as a guide for coming
up with the correct mangling string.

[reames]

Comments inline. I like the general direction of the revised patch.

lib/IR/Function.cpp

450 ↗

(On Diff #17995)

Please separate line breaking differences into their own patch. It's hard to tell what actually changed in this comment.

459 ↗

(On Diff #17995)

x88mmx isn't what actually appears in your test? Why the difference? (Clarify in comment please.)

FYI - I expect you'll run into some problems actually lowering vectors gc.relocates. It's not a case we've explored.

test/CodeGen/Generic/overloaded-intrinsic-name.ll

1 ↗

(On Diff #17995)

This is probably better as an opt -verify test. Not sure CodeGen is the right place for it either.

Alternatively, you could go the other direction and adapt this to be a gc lowering specific test. This would involve checking the assembly output, but might be reasonable tor exercising the vector path it seems like you're interested in.

4 ↗

(On Diff #17995)

You might want to clarify that the use of gc.* is arbitrary and that these are no GC tests. It's mostly clear, but could be a bit more explicit.

7 ↗

(On Diff #17995)

Given that these don't have check statements, how do you judge correctness of the tests?

[artagnon]

lib/IR/Function.cpp

459 ↗

(On Diff #17995)

x86mmx is an x86-specific type that's actually a 64-bit vector. There are already tests specifically for mmx in the same directory; I just want to document common name manglings in one place.

Yeah, I'll look into gc.relocate and vectors after this.

test/CodeGen/Generic/overloaded-intrinsic-name.ll

1 ↗

(On Diff #17995)

opt -verify -S it is then. Why not codegen? I can see plenty of similar code in the same directory.

I'll add to statepoint-call-lowering in subsequent patches.

7 ↗

(On Diff #17995)

We just want to ensure that it runs fine. Specifically checking name mangling against the types of arguments is not something I'm aiming for in this patch.

[artagnon]

Fix nits. Mostly the same.

[reames]

test/CodeGen/Generic/overloaded-intrinsic-name.ll

1 ↗

(On Diff #17995)

I have to admit, I didn't look at the nearby tests. If they had a particular style for testing the same thing, feel free to keep that.

7 ↗

(On Diff #17995)

Fine. Add a comment to that effect please? Top of the file is fine.

[artagnon]

From reames review: Add a comment on top of the test file stating its
purpose.

llvm/trunk/lib/IR/Function.cpp

 /// of named types is simply their name.  Manglings for unnamed types consist
 /// of a prefix ('p' for pointers, 'a' for arrays, 'f_' for functions)
 /// combined with the mangling of their component types.  A vararg function
 /// type will have a suffix of 'vararg'.  Since function types can contain
 /// other function types, we close a function type mangling with suffix 'f'
 /// which can't be confused with it's prefix.  This ensures we don't have
 /// collisions between two unrelated function types. Otherwise, you might
 /// parse ffXX as f(fXX) or f(fX)X.  (X is a placeholder for any other type.)
+/// Manglings of integers, floats, and vectors ('i', 'f', and 'v' prefix in most
+/// cases) fall back to the MVT codepath, where they could be mangled to
+/// 'x86mmx', for example; matching on derived types is not sufficient to mangle
+/// everything.
 static std::string getMangledTypeStr(Type* Ty) {
   std::string Result;
   if (PointerType* PTyp = dyn_cast<PointerType>(Ty)) {
     Result += "p" + llvm::utostr(PTyp->getAddressSpace()) +
       getMangledTypeStr(PTyp->getElementType());
   } else if (ArrayType* ATyp = dyn_cast<ArrayType>(Ty)) {
     Result += "a" + llvm::utostr(ATyp->getNumElements()) +
       getMangledTypeStr(ATyp->getElementType());

llvm/trunk/test/CodeGen/Generic/overloaded-intrinsic-name.ll

+; RUN: opt -verify -S < %s
+
+; Tests the name mangling performed by the codepath following
+; getMangledTypeStr(). Only tests that code with the various manglings
+; run fine: doesn't actually test the mangling with the type of the
+; arguments. Meant to serve as an example-document on how the user
+; should do name manglings.
+
+; Exercise the most general case, llvm_anyptr_type, using gc.relocate
+; and gc.statepoint. Note that it has nothing to do with gc.*
+; functions specifically: any function that accepts llvm_anyptr_type
+; will serve the purpose.
+
+; function and integer
+define i32* @test_iAny(i32* %v) {
+       %tok = call i32 (i1 ()*, i32, i32, ...)* @llvm.experimental.gc.statepoint.p0f_i1f(i1 ()* @return_i1, i32 0, i32 0, i32 0, i32* %v)
+       %v-new = call i32* @llvm.experimental.gc.relocate.p0i32(i32 %tok, i32 4, i32 4)
+       ret i32* %v-new
+}
+
+; float
+define float* @test_fAny(float* %v) {
+       %tok = call i32 (i1 ()*, i32, i32, ...)* @llvm.experimental.gc.statepoint.p0f_i1f(i1 ()* @return_i1, i32 0, i32 0, i32 0, float* %v)
+       %v-new = call float* @llvm.experimental.gc.relocate.p0f32(i32 %tok, i32 4, i32 4)
+       ret float* %v-new
+}
+
+; array of integers
+define [3 x i32]* @test_aAny([3 x i32]* %v) {
+       %tok = call i32 (i1 ()*, i32, i32, ...)* @llvm.experimental.gc.statepoint.p0f_i1f(i1 ()* @return_i1, i32 0, i32 0, i32 0, [3 x i32]* %v)
+       %v-new = call [3 x i32]* @llvm.experimental.gc.relocate.p0a3i32(i32 %tok, i32 4, i32 4)
+       ret [3 x i32]* %v-new
+}
+
+; vector of integers
+define <3 x i32>* @test_vAny(<3 x i32>* %v) {
+       %tok = call i32 (i1 ()*, i32, i32, ...)* @llvm.experimental.gc.statepoint.p0f_i1f(i1 ()* @return_i1, i32 0, i32 0, i32 0, <3 x i32>* %v)
+       %v-new = call <3 x i32>* @llvm.experimental.gc.relocate.p0v3i32(i32 %tok, i32 4, i32 4)
+       ret <3 x i32>* %v-new
+}
+
+declare zeroext i1 @return_i1()
+declare i32 @llvm.experimental.gc.statepoint.p0f_i1f(i1 ()*, i32, i32, ...)
+declare i32* @llvm.experimental.gc.relocate.p0i32(i32, i32, i32)
+declare float* @llvm.experimental.gc.relocate.p0f32(i32, i32, i32)
+declare [3 x i32]* @llvm.experimental.gc.relocate.p0a3i32(i32, i32, i32)
+declare <3 x i32>* @llvm.experimental.gc.relocate.p0v3i32(i32, i32, i32)