-
Notifications
You must be signed in to change notification settings - Fork 18
Split declaration statements #19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Split declaration statements with ExternalRewriter (version 1). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
По поводу определения позиции типа в начале объявления.
Есть еще такая конструкция, как clang::TypeLoc
, она позволяет отследить позиции типа в программе, т.е. можно описать, например, bool VisitTypeLoc(TypeLoc Loc) { ... }
или TraverseTypeLoc
.
Когда обрабатывается VarDecl
обходится тип, который ее формирует от самого широкого к самому узкому, например, для int *
, сначала будет посещен int *
, а потом int
.
Есть предположение, что если для VarDecl взять самый последний посещенный тип (TypeLoc), то это будет тот тип, который стоит перед объявлением, который общий для всех объявлений. И тогда, чтобы вычислить префикс, который соответствует типу можно взять диапазон от начала объявления до конца этого типа (именно от начала объявления, т.к. квалификаторы типа const не попадут в этот тип, но должны остаться).
Тогда должно быть все известно, что удалить лишнее и строки объявления.
Нужно еще обратить внимание, что не все объявления можно разбить, например,
struct {int X;} A, B;
A = B;
нельзя превратить в
struct {int X;} A,;
struct {int X;} B;
A = B;
т.к. в первом случае типы A
и B
одинаковые, а во втором уже нет.
Это относится как минимум к struct
, class
, union
, enum
.
Общие моменты
Сейчас как я понимаю допускаются директивы как на область видимости, так и на отдельный оператор? Можно так оставить. Но нужно проверять, что директива стоит в допустимом месте, т.е. нельзя ставить директиву перед любым оператором.
Хотелось бы, чтобы в конечно счете был отдельный метод, который можно было бы вызвать, чтобы обработать конкретное объявление переменных и разбить его. Тогда это позволит автоматически разбивать операторы объявления, при формировании параллельных версий программ в SAPFOR, когда это мешает поставить директиву распараллеливания. Возможно даже внутри этого метода стоит запускать отдельный Visitor, чтобы обойти все объявления в обрабатываемом операторе.
Т.е. есть два направления - разбить по директиве пользователя и по запросу от распараллеливателя в процессе генерации параллельной версии.
namespace llvm { | ||
/// This pass separates variable declaration statements that contain multiple | ||
/// variable declarations at once into single declarations. | ||
class ClangSplitDeclsPass : public ModulePass, private bcl::Uncopyable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Можно перенести объявление прохода в .cpp
файл, а .h
файл вообще удалить. Т.к. проход преобразования, не может быть запрошен из другого прохода, то и интерфейс его стоит скрыть внутри .cpp
файлы, оставив видимыми наружу только функции initialize...
и create...
объявленные в Passes.h
.
lib/Transform/Clang/SplitDecls.cpp
Outdated
auto &TfmInfo = getAnalysis<TransformationEnginePass>(); | ||
auto *TfmCtx{TfmInfo ? TfmInfo->getContext(M) : nullptr}; | ||
if (!TfmCtx || !TfmCtx->hasInstance()) { | ||
M.getContext().emitError("can not transform sources" | ||
": transformation context is not available"); | ||
return false; | ||
} | ||
ASTImportInfo ImportStub; | ||
const auto *ImportInfo = &ImportStub; | ||
if (auto *ImportPass = getAnalysisIfAvailable<ImmutableASTImportInfoPass>()) | ||
ImportInfo = &ImportPass->getImportInfo(); | ||
auto &GIP = getAnalysis<ClangGlobalInfoPass>(); | ||
ClangSplitter Vis(*TfmCtx, *ImportInfo, GIP.getRawInfo()); | ||
Vis.TraverseDecl(TfmCtx->getContext().getTranslationUnitDecl()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Это устаревший способ получить TransformationContext
, он не будет работать с многофайловыми проектами. Сейчас более правильный способ почти нигде не используется в master
, но с последующим большим обновлением, которое будет в конце этого кода, или начале следующего, это устаревший способ перестанет работать.
Правильный вариант для модульного прохода примерно такой:
auto &TfmInfo{getAnalysis<TransformationEnginePass>()};
if (!TfmInfo) {
M.getContext().emitError("cannot transform sources"
": transformation context is not available");
return false;
}
ASTImportInfo ImportStub;
const auto *ImportInfo = &ImportStub;
if (auto *ImportPass = getAnalysisIfAvailable<ImmutableASTImportInfoPass>())
ImportInfo = &ImportPass->getImportInfo();
auto &GIP{getAnalysis<ClangGlobalInfoPass>()};
auto *CUs{M.getNamedMetadata("llvm.dbg.cu")};
for (auto *MD : CUs->operands()) {
auto *CU{cast<DICompileUnit>(MD)};
auto *TfmCtx{
dyn_cast_or_null<ClangTransformationContext>(TfmInfo->getContext(*CU))};
if (!TfmCtx || !TfmCtx->hasInstance()) {
M.getContext().emitError("cannot transform sources"
": transformation context is not available");
return false;
}
// Здесь уже может быть ваш визитор ClangSplitter.
// Объемлющий цикл фактически обходит все файлы,
// входящие в проект, и для каждого получает свой контекст.
}
lib/Transform/Clang/SplitDecls.cpp
Outdated
bool isNotSingleFlag = false; | ||
bool isAfterNotSingleFlag = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Глобальные переменные - это плохо, переменные должны быть членами класса.
lib/Transform/Clang/SplitDecls.cpp
Outdated
bool TraverseDeclStmt(DeclStmt *S) { | ||
bool tmp; | ||
if(!(S->isSingleDecl())) { | ||
if (!isNotSingleFlag) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А в каком случае isNotSingleFlag
будет true
в этой точке?
lib/Transform/Clang/SplitDecls.cpp
Outdated
} | ||
|
||
bool TraverseDeclStmt(DeclStmt *S) { | ||
bool tmp; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bool tmp; |
lib/Transform/Clang/SplitDecls.cpp
Outdated
SourceRange varDeclRange(S->getBeginLoc(), S->getEndLoc()); | ||
if (varDeclsNum == 1) { | ||
SourceRange toInsert2(Range.getBegin(), S->getEndLoc()); | ||
txtStr = Canvas.getRewrittenText(varDeclRange).str(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Вроде бы нет причин делать txtStr
членом класса, она может быть локальной для метода.
lib/Transform/Clang/SplitDecls.cpp
Outdated
} | ||
} | ||
|
||
// bool VisitVarDecl(VarDecl *S) { // to traverse the parse tree and visit each statement |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Объявление:
int (*f)(int N, int(*C)[N]), x, (*restrict Q)[100], C[200][300];
Типы:
DeclStmt 0x55df67170ac8 <line:3:3, col:66>
|-VarDecl 0x55df67170668 <col:3, col:29> col:9 f 'int (*)(int, int (*)[N])'
|-VarDecl 0x55df671706e8 <col:3, col:32> col:32 x 'int'
|-VarDecl 0x55df67170898 <col:3, col:52> col:46 Q 'int (*restrict)[100]'
`-VarDecl 0x55df67170a38 <col:3, col:65> col:55 C 'int [200][300]'
Не получится вставить имя сразу после типа, например, для переменной f
(функция).
Не получится вставить имя на место последнего пробела, например, для переменной Q
(массив C99).
lib/Transform/Clang/SplitDecls.cpp
Outdated
if (isNotSingleFlag) { | ||
SourceRange toInsert(notSingleDeclStart, notSingleDeclEnd); | ||
mRewriter.RemoveText(toInsert, RemoveEmptyLine); | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Удаление исходного объявления лучше перенести поближе к обработке конкретного оператора объявления, например, делать это в TraverseDeclStmt
, сразу как оператор был обработан.
lib/Transform/Clang/SplitDecls.cpp
Outdated
bool HasMacro = false; | ||
for_each_macro(S, mSrcMgr, mContext.getLangOpts(), mRawInfo->Macros, | ||
[&HasMacro, this](clang::SourceLocation Loc) { | ||
if (!HasMacro) { | ||
toDiag(mContext.getDiagnostics(), Loc, | ||
tsar::diag::warn_splitdeclaration_macro_prevent); | ||
HasMacro = true; | ||
} | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Нет смысла проверять отсутствие макросов во все области видимости, т.к. обработка делается только для единичных операторов объявления и только, если эти операторы содержат макрос, то нужно запрещать их обработку.
Сейчас кстати судя по всему этой проверки нет, если прагма стоит на один оператор а не на область видимости.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Все еще актуально.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Убрала проверку на отсутствие макросов во всей области видимости
Добавила TraverseTypeLoc. В этой версии берётся самый последний посещенный тип и добавляется перед всеми объявлениями переменных. Такой подход не пригоден для некоторых типов, в частности для структур. У Type есть метод isStructureType(). Можно попробовать обрабатывать такие типы отдельно, но даже в таком случае брать первый, а не последний посещённый тип будет нельзя (пример: struct S * x, y). |
Есть тип Брать диапазон |
Реализация с ElaboratedTypeLoc. Теперь разделение объявлений работает со структурами и константами. Также была проблема с разделением объявлений массивов, сейчас она исправлена. Исправлена некорректная обработка прагмы, теперь разделение переменных происходит только в случае наличия строки #pragma spf transform splitdeclaration. |
Устаревший способ получения TransformationContext заменён на новый. Убраны глобальные переменные, теперь их роль играют переменные, являющиеся членами класса. Удалены переменные и функция, которые больше не используются. |
Исправлена некорректная работа программы на примерах с несколькими составными операторами объявления переменных. |
Реализация разделения на одиночные составных операторов инициализации переменных. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Общие замечания:
- Форматирование кода: длина строк, правила именования типов и переменных, закрытых членов класс и т.д. (подробнее в TSAR Wiki).
- Для всех ситуаций, которые не обрабатываются должно быть добавлено диагностическое сообщение о причине, почему преобрзование невозможно.
- Нужно предусмотреть возможность, которая позволила бы ограничить множество преобразуемых объявлений. Можно считать, что будет какой-то дополнительный проход, к оторому будет обращаться ваш проход и у которого он получить список
VarDecl
, которые нужно вынести в отдельный оператор. Т.е. Нужно предсмотреть, что параметром вашего визитора будетllvm::DenseSet<VarDecl *>
, и разделять нужно только эти объявления.
lib/Transform/Clang/CMakeLists.txt
Outdated
DeadDeclsElimination.cpp Format.cpp OpenMPAutoPar.cpp | ||
SharedMemoryAutoPar.cpp DVMHSMAutoPar.cpp StructureReplacement.cpp | ||
LoopInterchange.cpp LoopReversal.cpp SplitDecls.cpp) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Надо поправить, в таком виде не собирается, видимо после устранения конфликтов при rebase могли продублироваться имена файлов.
Vis.TraverseDecl(TfmCtx->getContext().getTranslationUnitDecl()); | ||
return false; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} | |
} | |
lib/Transform/Clang/SplitDecls.cpp
Outdated
if (auto *ImportPass = getAnalysisIfAvailable<ImmutableASTImportInfoPass>()) | ||
ImportInfo = &ImportPass->getImportInfo(); | ||
auto &GIP = getAnalysis<ClangGlobalInfoPass>(); | ||
//mGlobalInfo = &GIP.getGlobalInfo(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
//mGlobalInfo = &GIP.getGlobalInfo(); |
lib/Transform/Clang/SplitDecls.cpp
Outdated
private: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private: | |
private: |
lib/Transform/Clang/SplitDecls.cpp
Outdated
|
||
private: | ||
|
||
TransformationContext *mTfmCtx; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TransformationContext *mTfmCtx; | |
ClangTransformationContext *mTfmCtx; |
lib/Transform/Clang/SplitDecls.cpp
Outdated
bool HasMacro = false; | ||
for_each_macro(S, mSrcMgr, mContext.getLangOpts(), mRawInfo->Macros, | ||
[&HasMacro, this](clang::SourceLocation Loc) { | ||
if (!HasMacro) { | ||
toDiag(mContext.getDiagnostics(), Loc, | ||
tsar::diag::warn_splitdeclaration_macro_prevent); | ||
HasMacro = true; | ||
} | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Все еще актуально.
lib/Transform/Clang/SplitDecls.cpp
Outdated
SourceRange toInsert(S->getBeginLoc(), S->getEndLoc()); | ||
ProcessGlobalDeclaration(S, toInsert); | ||
} | ||
if (localVarDecls.isNotSingleFlag) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Вроде бы если переменная глобальна, то эта проверка вообще не должна выполняться, не нужен ли здесь else if
вместо if
?
lib/Transform/Clang/SplitDecls.cpp
Outdated
if (mGlobalInfo.findOutermostDecl(S)) { | ||
if (globalVarDeclsMap[S->getBeginLoc()].varDeclsNum == 0) { | ||
std::map<clang::SourceLocation, notSingleDecl>::iterator it; | ||
for (it = globalVarDeclsMap.begin(); it != globalVarDeclsMap.end(); it++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Почему не использовать метод find()? Зачем использовать map, если поиск делается перебором его элементов?
- Вместо std::map, лучше использовать llvm::DenseMap.
- Применять многкратно оператор
[...]
для map нехорошо, каждое его применение - это поиск по в map, лучше найти однократно и затем использовать полученную ссылку/итератор на объект для доступа. - Для вставки удобно использовать метод try_emplace(), он сразу вернет пару итератор на новый объект или на уже существующий объект и флаг типа
bool
, указыавющий на то был ли объект добавлен или он уже был в контейнере.
lib/Transform/Clang/SplitDecls.cpp
Outdated
localVarDecls.notSingleDeclEnd = S->getEndLoc(); | ||
varPositions[S->getBeginLoc()] = S->getEndLoc(); | ||
} else { | ||
std::cout << "IS SINGLE\n"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В LLVM для вывода используются собственные потоки outs()
, errs()
, dbgs()
: outs() << ... << ...
. В финальной версии не должно быть отладочной печати, либо ее можно оставить, но окружить специальным макросом LLVM_DEBUG
: LLVM_DEBUG(dbgs() << "[SPLIT DECLARATIONS]: ...\n")
. В этом случае отладочная печать будет доступна при сборке системы в Debug
по опции -debug-only=clang-split-decls (имя опции определяется макросом `#define DEBUG_TYPE "clang-split-decls" в начале .cpp файла).
return type; | ||
} | ||
|
||
bool TraverseTypeLoc(TypeLoc Loc) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Мне казалось раньше была отдельная обработка структурных типов?
Сейчас struct {int X;} A, B;
обрабатывается неправильно.
А что должно быть для такого случая: int (*f)(int N, int(*C)[N]), x, (*restrict Q)[100], C[200][300];, сейчас он не преобразуется.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Корректно обрабатывается struct S {int X;} A, B;
. Ранее мы решили случаи типа int (*f)(int N, int(*C)[N]), x, (*restrict Q)[100], C[200][300];
не обрабатывать, такие объявления отлавливаются при помощи bool VisitParmVarDecl(ParmVarDecl *S)
Проведён рефакторинг кода в соответствии с правилами оформления кода. Для ситуаций, которые не обрабатываются, добавлены диагностические сообщения. Устранены многократные применения оператора |
…t initializations of global variables
Removed processing of all global variable declarations in each iteration. Now, each global variable declaration is processed only once.
…iagnostic messages.
ccd3bca
to
ac3ce33
Compare
No description provided.