Page MenuHomePhabricator

[OpenMP] Implement '#pragma omp unroll'.
Needs ReviewPublic

Authored by Meinersbur on Mar 27 2021, 4:33 PM.

Details

Summary

Implementation of the unroll directive introduced in OpenMP 5.1. Follows the approach from D76342 for the tile directive (i.e. AST-based, not using the OpenMPIRBuilder). Tries to use llvm.loop.unroll.* metadata where possible, but has to fall back to an AST representation of the outer loop if the partially unrolled generated loop is associated with another directive (because it needs to compute the number of iterations).

Diff Detail

Event Timeline

Meinersbur created this revision.Mar 27 2021, 4:33 PM
Meinersbur requested review of this revision.Mar 27 2021, 4:33 PM
ABataev added inline comments.
clang/include/clang/AST/OpenMPClause.h
899

sizes->full

902

Same

903

No param

904

Wrong description

905

No param

912

No param

946

sizes->partial

948–952

Fix params descriptions

957–960

Description

971

Make it private, if possible

clang/include/clang/AST/StmtOpenMP.h
5070

Description

5094–5105

Fix description

5111–5115

Description

clang/lib/AST/StmtProfile.cpp
466

Unelated change?

474

const Expr *

clang/lib/CodeGen/CGStmtOpenMP.cpp
2589

getSingleClause or hasClausesOfKind

2596

getSingleClause

2605

const Expr *

2606–2609

I suppose it is compiled-time expression, right? If so, use FactorExpr->EvaluateKnownConstInt() or something similar.

clang/lib/Parse/ParseOpenMP.cpp
2973–2974

Better to remove the default: case completely

clang/lib/Sema/SemaOpenMP.cpp
12558

I think this function can be made static local as it is used only SemaOpenMP.cpp

12822

getSingleClause

12825

Add a text message to the assert.

12830

getSingleClause

12848

Need to remove

12855–12857

I think this should be checked before the first OMPUnrollDirective::Create call

12931

unroll?

14963

Need to pass factor expr and real source loc

clang/lib/Serialization/ASTReaderStmt.cpp
3194

Add a message for the assert

  • Ready version
  • Undo spurious change
Meinersbur retitled this revision from [OpenMP] Implement '#pragma omp unroll'. WIP. to [OpenMP] Implement '#pragma omp unroll'..Wed, May 12, 9:09 PM
Meinersbur edited the summary of this revision. (Show Details)

Removed [WIP] flag; ready to review.

Meinersbur marked 29 inline comments as done.Wed, May 12, 9:11 PM

Some previous comments were not addressed yet

clang/include/clang/AST/OpenMPClause.h
972–974

Make it private

clang/lib/AST/StmtOpenMP.cpp
141

Enclose in braces

clang/lib/CodeGen/CGStmtOpenMP.cpp
2600–2604

I suppose it is compiled-time expression, right? If so, use FactorExpr->EvaluateKnownConstInt() or something similar.