Page MenuHomePhabricator

Clang support for simd functions
Needs ReviewPublic

Authored by mmasten on Nov 28 2017, 1:36 PM.

Details

Summary

This patch adds "vector-variants" function attributes to simd functions.

Diff Detail

Event Timeline

mmasten created this revision.Nov 28 2017, 1:36 PM

This patch is related to revisions D22792 and D40575

Hahnfeld added a subscriber: Hahnfeld.

This should add or extend a regression test

mmasten updated this revision to Diff 194917.Apr 12 2019, 10:53 AM

Rebased and updated test.

ABataev added inline comments.Apr 12 2019, 11:02 AM
clang/lib/CodeGen/CGOpenMPRuntime.cpp
9615
  1. Why this change is required?
  2. Why not a SmallString?
clang/test/OpenMP/declare_simd_codegen.cpp
323

Hmm, a very strange update of the test, just quotes are removed and nothing else.

mmasten marked 2 inline comments as done.Apr 12 2019, 11:18 AM
mmasten added inline comments.
clang/lib/CodeGen/CGOpenMPRuntime.cpp
9615

This change is required to more clearly indicate which function names represent simd function variants of the original scalar function. Previously, these function names were just represented as individual strings. Example: "_ZGVbM4l8Z5add_1Pf", "_ZGVbN4l8Z5add_1Pf", ... The attributes are now represented as a key/value pair: "vector-variants"="_ZGVbM4l8Z5add_1Pf, _ZGVbN4l8Z5add_1Pf, ...". Because the length of the string can now be quite a bit longer, I used std::string. Is SmallString still appropriate?

clang/test/OpenMP/declare_simd_codegen.cpp
323

I can probably change the test to be more explicit in checking for "vector-variants", if you think this would be better. The quotes were removed because of the change to the attribute formatting. E.g., the function names will no longer appear as individual strings, but as the key/value pair where all function names will now appear within a single string.

ABataev added inline comments.Apr 12 2019, 11:49 AM
clang/lib/CodeGen/CGOpenMPRuntime.cpp
9615
  1. I would suggest to try to change this part of the compiler only after the LLVM part is accepted to avoid double work here.
  2. Yes, SmallString, SmallVector, etc. are dynamically resized if the preallocated space is not enough.