Please review codegen of the ‘aligned’ clause for the ‘omp simd’ directive.
It uses @llvm.assume intrinsic to specify alignment hints in LLVM IR.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
include/clang/Basic/DiagnosticSemaKinds.td | ||
---|---|---|
7180 ↗ | (On Diff #14097) | I don't like the " - ignored" part, how about rewording like this: "aligned clause will be ignored because the requested alignment is not a power of 2" |
lib/CodeGen/CGStmtOpenMP.cpp | ||
53 ↗ | (On Diff #14097) | Remove this (see below). |
76 ↗ | (On Diff #14097) | I don't think that using Safelen here is the right thing to do; a loop can have a very large safelen without that implying anything about the alignment of the pointers. The right thing to do here is to use a callback into TargetCodeGenInfo (which you can get at using the TheTargetCodeGenInfo global). |
105 ↗ | (On Diff #14097) | Remove the FIXME. |
Hi Hal,
Thank you for review, I've fixed your comments. Here is updated version.
I added method getOpenMPSimdDefaultAlignment to TheTargetCodeGenInfo (implemented it for x86_64 only for now).
Regards,
Alexander
lib/CodeGen/CGStmtOpenMP.cpp | ||
---|---|---|
64 ↗ | (On Diff #14157) | I think that you should just assert that this is either a power of 2 (or zero): if a target starts returning some other kind of number, that would be a bug, and we'd want to know. |
69 ↗ | (On Diff #14157) | As I also mention below, when we use a default alignment, I'll need that to depend on the type, so you'll need to rearrange things here slightly. |
94 ↗ | (On Diff #14157) | You don't need the {} here. |
lib/CodeGen/TargetInfo.h | ||
223 ↗ | (On Diff #14157) | I don't really like this description; how about this: /// Gets the target-specific default alignment used when an 'aligned' clause is used with a 'simd' OpenMP directive without specifying a specific alignment. |
224 ↗ | (On Diff #14157) | In general, I'll need this to depend on the type; please add a type parameter. |
Hal,
Here is updated patch, I added type parameter to getOpenMPSimdDefaultAlignment (I guess you meant llvm::Type, not clang::Type) and fixed other issues.
Thanks,
Alexander
I added type parameter to getOpenMPSimdDefaultAlignment (I guess you meant llvm::Type, not clang::Type)
I think I'd actually like the QualType (which I suppose is just E->getType()), so that pointers to single-element structures can be handled like pointers to the underlying type in a straightforward way (meaning using the same logic that already exists in the ABI code).
Otherwise, LGTM. Thanks!