Skip to content
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

Remove-firstprivate pass #18

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

NikitaGorbov
Copy link

No description provided.

@NikitaGorbov
Copy link
Author

Добавил инициализацию массивов

Copy link
Member

@kaniandr kaniandr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Добавил некоторые замечания и пояснения по методам и классам.

lib/Transform/Clang/RemoveFirstPrivate.cpp Outdated Show resolved Hide resolved
include/tsar/Transform/Clang/RemoveFirstPrivate.h Outdated Show resolved Hide resolved
lib/Transform/Clang/RemoveFirstPrivate.cpp Outdated Show resolved Hide resolved
lib/Transform/Clang/RemoveFirstPrivate.cpp Outdated Show resolved Hide resolved
lib/Transform/Clang/RemoveFirstPrivate.cpp Outdated Show resolved Hide resolved
lib/Transform/Clang/RemoveFirstPrivate.cpp Outdated Show resolved Hide resolved
lib/Transform/Clang/RemoveFirstPrivate.cpp Outdated Show resolved Hide resolved
@NikitaGorbov
Copy link
Author

Удалил .h файл, избавился от функции, получающей строку по позиции в коде (вместо неё теперь используются методы getName() и getValue()). Изменил способ получения TransformationContext. Исправил объявления функций (добавил static) и структуры (перенёс в безымянный namespace). Проход теперь не создаёт указатели, а добавляет к имени массива квадратные скобки.

@NikitaGorbov
Copy link
Author

NikitaGorbov commented Dec 9, 2021

Добавил возможность инициализации многомерных массивов.
Пример:

int arr1[5][2], arr2[5][2];
#pragma spf transform removefirstprivate(arr1 = arr2 5 2)

Развернётся в

for (int i0; i0 < 5; i0++) {
    for (int i1; i1 < 2; i1++) {
        arr1[i0][i1] = arr2[i0][i1];
     }
}

Инициализировать можно другим массивом, переменной или числом (каждый элемент приравнивается переменной/числу).
Пока инициализировать можно только многомерные массивы с фиксированными размерами (скоро сделаю для указателей)

@NikitaGorbov
Copy link
Author

Размеры массива для инициализации теперь определяются автоматически, если они не заданы вручную в клаузе. Теперь можно писать так:

int arr1[5][2], arr2[5][2];
#pragma spf transform removefirstprivate(arr1 = arr2)

И это развернётся в

for (int i0; i0 < 5; i0++) {
    for (int i1; i1 < 2; i1++) {
        arr1[i0][i1] = arr2[i0][i1];
     }
}

@NikitaGorbov
Copy link
Author

Добавил возможность инициализировать многомерные динамические массивы (размеры массива в этом случае необходимо указывать в клаузе).

Copy link
Member

@kaniandr kaniandr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1️⃣ Много замечаний по форматированию, я не стал отмечать все явно:

  • именование переменных, типов, и т.д.,
  • длина строк,
  • лишние пустые сроки и пробелы.

Какие-то вещи можно исправить автоматически, как в студии так и в VSCode есть возможность автоматического форматирования стиля в соответсвии с форматом LLVM, имена это не исправит, но сдвиги и длины строк поправит.

2️⃣ Кроме форматирования - не хватает диагностических сообщений, как о некоретности задания директивы, так и об отказе преобразовывать по каким-то другим причинам. Нужно исходить из того, что пользователь может написать все что угдно и прежде чем преобразовывать, должна быть проверена допустимость преобразования. Нужно добавить диагностики (см. tsar/Support/Diagnostics.td) и сделать их выдачу, используя метод toDiag(...).

3️⃣ Давайте изменим синтаксис и название директивы:

  #pragma spf transform initialize(I = J)
  #pragma spf transform initialize(I = J, [5][6])

@@ -115,4 +122,4 @@ void initializeClangLoopReversePass(PassRegistry &Registry);
/// Create a pass to reverse loop.
ModulePass *createClangLoopReverse();
}
#endif//TSAR_CLANG_TRANSFORM_PASSES_H
#endif//TSAR_CLANG_TRANSFORM_PASSES_H
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Нужно убрать изменения строк, которые не были затронуты (символы завершения строки).

lib/Transform/Clang/RemoveFirstPrivate.cpp Outdated Show resolved Hide resolved
lib/Transform/Clang/RemoveFirstPrivate.cpp Outdated Show resolved Hide resolved
lib/Transform/Clang/RemoveFirstPrivate.cpp Outdated Show resolved Hide resolved
lib/Transform/Clang/RemoveFirstPrivate.cpp Outdated Show resolved Hide resolved
lib/Transform/Clang/RemoveFirstPrivate.cpp Outdated Show resolved Hide resolved
lib/Transform/Clang/RemoveFirstPrivate.cpp Outdated Show resolved Hide resolved
ast = RecursiveASTVisitor::TraverseStmt(S);
isInPragma = false;

std::string txtStr, beforeFor, forBody, lval, rval, indeces;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Эти строки скорее всего маленькие, лучше использовать llvm::SmallString<...>, чтобы избежать лишних выделений памяти в куче. Например, llvm::SmallString<64> выделт строку на стеке длиной 64 символа, если не хватит, то неявно превратится в обычный string.

lib/Transform/Clang/RemoveFirstPrivate.cpp Outdated Show resolved Hide resolved
lib/Transform/Clang/RemoveFirstPrivate.cpp Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants