Meinersbur accepted this revision.
Meinersbur added a comment.
This revision is now accepted and ready to land.
LGTM
================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:3042
+
+ if (!(Simdlen == nullptr && Safelen == nullptr)) {
+ // If both simdlen and safelen clauses are specified, the value of the
----------------
Consider simplifying this expression
================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:3048
+ // parameter. Therefore, use safelen only in the absence of simdlen.
+ ConstantInt *VectorizeWidth = Simdlen == nullptr ? Safelen : Simdlen;
addLoopMetadata(
----------------
psoni2628 wrote:
> Meinersbur wrote:
> > `safelen` should not mean the same as `llvm.loop.vectorize.width`.
> > `safelen` could be unreasonably large to use as SIMD width or a
> > non-power-of-2.
> >
> > That being said, it's what `CGStmtOpenMP.cpp` does as well and I don't know
> > any better way.
> IMO, this is more of a semantic analysis problem. For example, if it not
> legal to have a `safelen` that is a non-power-of-2, then Clang should not let
> this value proceed from semantic analysis. Maybe we could add a check in
> `clang/lib/Sema/SemaOpenMP.cpp`, and fix the problem for both OMPIRBuilder
> and the existing codegen support in `CGStmtOpenMP.cpp` in a different patch?
I think it's not a responsibility of Clang, but the optimizer. For instance,
the information that "infinite" vector width is good (#pragma clang loop
vectorize(assume_safety)` and `#pragma omp simd` without `safelen`) is passed
as metadata (`addSimdMetadata`), but there is not metadata for conveying that
only a specific vector length is safe.
There are other issues than just power-of-2, such as
* too-large safelen (e.g. 128 when the target only has simd instructions of
length 4).
* A vector width of 2 having better performance even though `safelen(2)` is
used.
* The LoopVectorize will not vectorize at all if it cannot conclude the
legality of it when only `!{!"llvm.loop.vectorize.width", i32 4}` is specified,
but not also the information that the user guarantees that it is safe to do so.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131526/new/
https://reviews.llvm.org/D131526
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
