Page MenuHomePhabricator

[mips] MSA intrinsics header file
ClosedPublic

Authored by sdardis on Sep 16 2016, 9:31 AM.

Details

Summary

This patch adds the msa.h header file containing the shorter names for the
MSA instrinsics, e.g. msa_sll_b for builtin_msa_sll_b.

Diff Detail

Event Timeline

sdardis updated this revision to Diff 71666.Sep 16 2016, 9:31 AM
sdardis retitled this revision from to [mips] MSA intrinsics header file.
sdardis updated this object.
sdardis added a subscriber: cfe-commits.
vkalintiris accepted this revision.Sep 20 2016, 6:12 AM
vkalintiris edited edge metadata.
vkalintiris added a subscriber: mpf.

LGTM with one comment/question inline.

test/CodeGen/builtins-mips-msa.c
8

@mpf

Would it be problematic to expose this typedef from msa.h?

This revision is now accepted and ready to land.Sep 20 2016, 6:12 AM
mpf added inline comments.Sep 20 2016, 8:04 AM
test/CodeGen/builtins-mips-msa.c
8

Yes it would unfortunately.

We don't have an __fp16 type in the MIPS ABIs so it doesn't really exist. The MSA builtins are in fact not defined in terms of half-float they are just represented as integers via v8i16. This is because there is very little you can do with the half-float type in general apart from the conversions.

I also notice in the test case that v8f16 is used in numerous places where it should be v4f32 and v4f32 where it should be v2f64 (I think). I'll highlight an example below.

361

This is a correct use of v8f16.

390

This is an incorrect use of v8f16. There are many more.

391

This should be v2f64 I believe, there are many more of these too.

@mpf
I'll correct the incorrect uses in a followup patch.

Thanks,
Simon

vkalintiris requested changes to this revision.Sep 20 2016, 9:01 AM
vkalintiris edited edge metadata.

@sdardis: can you update the review request before committing this because I don't think that test/CodeGen/builtins-mips-msa.c has ever been properly reviewed before.

This revision now requires changes to proceed.Sep 20 2016, 9:01 AM
vkalintiris accepted this revision.Sep 20 2016, 9:07 AM
vkalintiris edited edge metadata.

@sdardis: can you update the review request before committing this because I don't think that test/CodeGen/builtins-mips-msa.c has ever been properly reviewed before.

Never mind, I didn't realize that you intend to write a follow-up patch.

This revision is now accepted and ready to land.Sep 20 2016, 9:07 AM
sdardis closed this revision.Sep 20 2016, 9:24 AM

As stated I'll do a follow-up patch to fix the test. Committed as r281975.

Thanks,
Simon

Checking the generated IR for the incorrect uses of v8f16 shows that clang is silently generating bitcasts from <8 x i16> to <4 x float>. There are some other cases where the type of the operands is incorrect w.r.t. the builtin used.

I'm not familiar with the type checking machinery, so I'm unsure if the silent conversion is a bug or feature.