Skip to content

Commit e6e00ae

Browse files
eZWALTeZWALT
committed
Address basic PR feedback
1 parent ac0d9e3 commit e6e00ae

File tree

8 files changed

+166
-197
lines changed

8 files changed

+166
-197
lines changed

‎clang/include/clang/AST/OpenMPClause.h

Lines changed: 42 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1153,82 +1153,73 @@ class OMPFullClause final : public OMPNoChildClause<llvm::omp::OMPC_full> {
11531153
/// for(int j = 0; j < 256; j+=2)
11541154
/// for(int k = 127; k >= 0; --k)
11551155
/// \endcode
1156-
class OMPLoopRangeClause final : public OMPClause {
1156+
class OMPLoopRangeClause final
1157+
: public OMPClause,
1158+
private llvm::TrailingObjects<OMPLoopRangeClause, Expr *> {
11571159
friend class OMPClauseReader;
1158-
1159-
explicit OMPLoopRangeClause()
1160-
: OMPClause(llvm::omp::OMPC_looprange, {}, {}) {}
1160+
friend class llvm::TrailingObjects<OMPLoopRangeClause, Expr *>;
11611161

11621162
/// Location of '('
11631163
SourceLocation LParenLoc;
11641164

1165-
/// Location of 'first'
1166-
SourceLocation FirstLoc;
1167-
1168-
/// Location of 'count'
1169-
SourceLocation CountLoc;
1170-
1171-
/// Expr associated with 'first' argument
1172-
Expr *First = nullptr;
1173-
1174-
/// Expr associated with 'count' argument
1175-
Expr *Count = nullptr;
1176-
1177-
/// Set 'first'
1178-
void setFirst(Expr *First) { this->First = First; }
1165+
/// Location of first and count expressions
1166+
SourceLocation FirstLoc, CountLoc;
11791167

1180-
/// Set 'count'
1181-
void setCount(Expr *Count) { this->Count = Count; }
1168+
/// Number of looprange arguments (always 2: first, count)
1169+
unsigned NumArgs = 2;
11821170

1183-
/// Set location of '('.
1184-
void setLParenLoc(SourceLocation Loc) { LParenLoc = Loc; }
1185-
1186-
/// Set location of 'first' argument
1187-
void setFirstLoc(SourceLocation Loc) { FirstLoc = Loc; }
1171+
/// Set the argument expressions.
1172+
void setArgs(ArrayRef<Expr *> Args) {
1173+
assert(Args.size() == NumArgs && "Expected exactly 2 looprange arguments");
1174+
std::copy(Args.begin(), Args.end(), getTrailingObjects<Expr *>());
1175+
}
11881176

1189-
/// Set location of 'count' argument
1190-
void setCountLoc(SourceLocation Loc) { CountLoc = Loc; }
1177+
/// Build an empty clause for deserialization.
1178+
explicit OMPLoopRangeClause()
1179+
: OMPClause(llvm::omp::OMPC_looprange, {}, {}), NumArgs(2) {}
11911180

11921181
public:
1193-
/// Build an AST node for a 'looprange' clause
1194-
///
1195-
/// \param StartLoc Starting location of the clause.
1196-
/// \param LParenLoc Location of '('.
1197-
/// \param ModifierLoc Modifier location.
1198-
/// \param
1182+
/// Build a 'looprange' clause AST node.
11991183
static OMPLoopRangeClause *
12001184
Create(const ASTContext &C, SourceLocation StartLoc, SourceLocation LParenLoc,
12011185
SourceLocation FirstLoc, SourceLocation CountLoc,
1202-
SourceLocation EndLoc, Expr *First, Expr *Count);
1186+
SourceLocation EndLoc, ArrayRef<Expr *> Args);
12031187

1204-
/// Build an empty 'looprange' node for deserialization
1205-
///
1206-
/// \param C Context of the AST.
1188+
/// Build an empty 'looprange' clause node.
12071189
static OMPLoopRangeClause *CreateEmpty(const ASTContext &C);
12081190

1209-
/// Returns the location of '('
1191+
// Location getters/setters
12101192
SourceLocation getLParenLoc() const { return LParenLoc; }
1211-
1212-
/// Returns the location of 'first'
12131193
SourceLocation getFirstLoc() const { return FirstLoc; }
1214-
1215-
/// Returns the location of 'count'
12161194
SourceLocation getCountLoc() const { return CountLoc; }
12171195

1218-
/// Returns the argument 'first' or nullptr if not set
1219-
Expr *getFirst() const { return cast_or_null<Expr>(First); }
1196+
void setLParenLoc(SourceLocation Loc) { LParenLoc = Loc; }
1197+
void setFirstLoc(SourceLocation Loc) { FirstLoc = Loc; }
1198+
void setCountLoc(SourceLocation Loc) { CountLoc = Loc; }
12201199

1221-
/// Returns the argument 'count' or nullptr if not set
1222-
Expr *getCount() const { return cast_or_null<Expr>(Count); }
1200+
/// Get looprange arguments: first and count
1201+
Expr *getFirst() const { return getArgs()[0]; }
1202+
Expr *getCount() const { return getArgs()[1]; }
12231203

1224-
child_range children() {
1225-
return child_range(reinterpret_cast<Stmt **>(&First),
1226-
reinterpret_cast<Stmt **>(&Count) + 1);
1204+
/// Set looprange arguments: first and count
1205+
void setFirst(Expr *E) { getArgs()[0] = E; }
1206+
void setCount(Expr *E) { getArgs()[1] = E; }
1207+
1208+
MutableArrayRef<Expr *> getArgs() {
1209+
return MutableArrayRef<Expr *>(getTrailingObjects<Expr *>(), NumArgs);
1210+
}
1211+
ArrayRef<Expr *> getArgs() const {
1212+
return ArrayRef<Expr *>(getTrailingObjects<Expr *>(), NumArgs);
12271213
}
12281214

1215+
child_range children() {
1216+
return child_range(reinterpret_cast<Stmt **>(getArgs().begin()),
1217+
reinterpret_cast<Stmt **>(getArgs().end()));
1218+
}
12291219
const_child_range children() const {
1230-
auto Children = const_cast<OMPLoopRangeClause *>(this)->children();
1231-
return const_child_range(Children.begin(), Children.end());
1220+
auto AR = getArgs();
1221+
return const_child_range(reinterpret_cast<Stmt *const *>(AR.begin()),
1222+
reinterpret_cast<Stmt *const *>(AR.end()));
12321223
}
12331224

12341225
child_range used_children() {

‎clang/include/clang/AST/StmtOpenMP.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5807,7 +5807,6 @@ class OMPReverseDirective final : public OMPLoopTransformationDirective {
58075807
llvm::omp::OMPD_reverse, StartLoc,
58085808
EndLoc, 1) {
58095809
// Reverse produces a single top-level canonical loop nest
5810-
setNumGeneratedLoops(1);
58115810
setNumGeneratedLoopNests(1);
58125811
}
58135812

@@ -5878,7 +5877,7 @@ class OMPInterchangeDirective final : public OMPLoopTransformationDirective {
58785877
EndLoc, NumLoops) {
58795878
// Interchange produces a single top-level canonical loop
58805879
// nest, with the exact same amount of total loops
5881-
setNumGeneratedLoops(NumLoops);
5880+
setNumGeneratedLoops(3 * NumLoops);
58825881
setNumGeneratedLoopNests(1);
58835882
}
58845883

‎clang/include/clang/Sema/SemaOpenMP.h

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1491,7 +1491,7 @@ class SemaOpenMP : public SemaBase {
14911491
bool checkTransformableLoopNest(
14921492
OpenMPDirectiveKind Kind, Stmt *AStmt, int NumLoops,
14931493
SmallVectorImpl<OMPLoopBasedDirective::HelperExprs> &LoopHelpers,
1494-
Stmt *&Body, SmallVectorImpl<SmallVector<Stmt *, 0>> &OriginalInits);
1494+
Stmt *&Body, SmallVectorImpl<SmallVector<Stmt *>> &OriginalInits);
14951495

14961496
/// @brief Categories of loops encountered during semantic OpenMP loop
14971497
/// analysis
@@ -1554,9 +1554,9 @@ class SemaOpenMP : public SemaBase {
15541554
Stmt *LoopSeqStmt, unsigned &LoopSeqSize, unsigned &NumLoops,
15551555
SmallVectorImpl<OMPLoopBasedDirective::HelperExprs> &LoopHelpers,
15561556
SmallVectorImpl<Stmt *> &ForStmts,
1557-
SmallVectorImpl<SmallVector<Stmt *, 0>> &OriginalInits,
1558-
SmallVectorImpl<SmallVector<Stmt *, 0>> &TransformsPreInits,
1559-
SmallVectorImpl<SmallVector<Stmt *, 0>> &LoopSequencePreInits,
1557+
SmallVectorImpl<SmallVector<Stmt *>> &OriginalInits,
1558+
SmallVectorImpl<SmallVector<Stmt *>> &TransformsPreInits,
1559+
SmallVectorImpl<SmallVector<Stmt *>> &LoopSequencePreInits,
15601560
SmallVectorImpl<OMPLoopCategory> &LoopCategories, ASTContext &Context,
15611561
OpenMPDirectiveKind Kind);
15621562

@@ -1590,9 +1590,9 @@ class SemaOpenMP : public SemaBase {
15901590
unsigned &NumLoops,
15911591
SmallVectorImpl<OMPLoopBasedDirective::HelperExprs> &LoopHelpers,
15921592
SmallVectorImpl<Stmt *> &ForStmts,
1593-
SmallVectorImpl<SmallVector<Stmt *, 0>> &OriginalInits,
1594-
SmallVectorImpl<SmallVector<Stmt *, 0>> &TransformsPreInits,
1595-
SmallVectorImpl<SmallVector<Stmt *, 0>> &LoopSequencePreInits,
1593+
SmallVectorImpl<SmallVector<Stmt *>> &OriginalInits,
1594+
SmallVectorImpl<SmallVector<Stmt *>> &TransformsPreInits,
1595+
SmallVectorImpl<SmallVector<Stmt *>> &LoopSequencePreInits,
15961596
SmallVectorImpl<OMPLoopCategory> &LoopCategories, ASTContext &Context);
15971597

15981598
/// Helper to keep information about the current `omp begin/end declare

‎clang/lib/AST/OpenMPClause.cpp

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1026,22 +1026,25 @@ OMPPartialClause *OMPPartialClause::CreateEmpty(const ASTContext &C) {
10261026

10271027
OMPLoopRangeClause *
10281028
OMPLoopRangeClause::Create(const ASTContext &C, SourceLocation StartLoc,
1029-
SourceLocation LParenLoc, SourceLocation EndLoc,
1030-
SourceLocation FirstLoc, SourceLocation CountLoc,
1031-
Expr *First, Expr *Count) {
1029+
SourceLocation LParenLoc, SourceLocation FirstLoc,
1030+
SourceLocation CountLoc, SourceLocation EndLoc,
1031+
ArrayRef<Expr *> Args) {
1032+
1033+
assert(Args.size() == 2 &&
1034+
"looprange clause must have exactly two arguments");
10321035
OMPLoopRangeClause *Clause = CreateEmpty(C);
10331036
Clause->setLocStart(StartLoc);
10341037
Clause->setLParenLoc(LParenLoc);
1035-
Clause->setLocEnd(EndLoc);
10361038
Clause->setFirstLoc(FirstLoc);
10371039
Clause->setCountLoc(CountLoc);
1038-
Clause->setFirst(First);
1039-
Clause->setCount(Count);
1040+
Clause->setLocEnd(EndLoc);
1041+
Clause->setArgs(Args);
10401042
return Clause;
10411043
}
10421044

10431045
OMPLoopRangeClause *OMPLoopRangeClause::CreateEmpty(const ASTContext &C) {
1044-
return new (C) OMPLoopRangeClause();
1046+
void *Mem = C.Allocate(totalSizeToAlloc<Expr *>(2));
1047+
return new (Mem) OMPLoopRangeClause();
10451048
}
10461049

10471050
OMPAllocateClause *OMPAllocateClause::Create(

‎clang/lib/CodeGen/CGExpr.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3241,11 +3241,8 @@ LValue CodeGenFunction::EmitDeclRefLValue(const DeclRefExpr *E) {
32413241
var, ConvertTypeForMem(VD->getType()), getContext().getDeclAlign(VD));
32423242

32433243
// No other cases for now.
3244-
} else {
3245-
llvm::dbgs() << "THE DAMN DECLREFEXPR HASN'T BEEN ENTERED IN LOCALDECLMAP\n";
3246-
VD->dumpColor();
3244+
} else
32473245
llvm_unreachable("DeclRefExpr for Decl not entered in LocalDeclMap?");
3248-
}
32493246

32503247
// Handle threadlocal function locals.
32513248
if (VD->getTLSKind() != VarDecl::TLS_None)

‎clang/lib/CodeGen/CodeGenFunction.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5414,10 +5414,6 @@ class CodeGenFunction : public CodeGenTypeCache {
54145414

54155415
/// Set the address of a local variable.
54165416
void setAddrOfLocalVar(const VarDecl *VD, Address Addr) {
5417-
if (LocalDeclMap.count(VD)) {
5418-
llvm::errs() << "Warning: VarDecl already exists in map: ";
5419-
VD->dumpColor();
5420-
}
54215417
assert(!LocalDeclMap.count(VD) && "Decl already exists in LocalDeclMap!");
54225418
LocalDeclMap.insert({VD, Addr});
54235419
}

0 commit comments

Comments
 (0)