Page MenuHomePhabricator

[OMPIRBuilder] Add support for simd (loop) directive.
Needs RevisionPublic

Authored by arnamoy10 on Mon, Nov 22, 9:39 AM.

Details

Summary

This patch adds OMPIRBuilder support for the simd directive (without any clause). This will be a first step towards lowering simd directive in LLVM_Flang. The patch uses existing CanonicalLoop infrastructure of IRBuilder to add the support. Also adds necessary code to add llvm.access.group and llvm.loop metadata wherever needed.

Diff Detail

Unit TestsFailed

TimeTest
470 msx64 debian > Clang.OpenMP::ordered_codegen.cpp
Script: -- : 'RUN: at line 2'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -cc1 -internal-isystem /var/lib/buildkite-agent/builds/llvm-project/build/lib/clang/14.0.0/include -nostdsysteminc -verify -fopenmp -x c++ -triple x86_64-unknown-unknown -emit-llvm /var/lib/buildkite-agent/builds/llvm-project/clang/test/OpenMP/ordered_codegen.cpp -fexceptions -fcxx-exceptions -o - | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck --allow-unused-prefixes /var/lib/buildkite-agent/builds/llvm-project/clang/test/OpenMP/ordered_codegen.cpp --check-prefixes=CHECK1,CHECK1-NORMAL

Event Timeline

arnamoy10 created this revision.Mon, Nov 22, 9:39 AM
arnamoy10 requested review of this revision.Mon, Nov 22, 9:39 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript
arnamoy10 edited the summary of this revision. (Show Details)Mon, Nov 22, 9:40 AM
tschuett added inline comments.
llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
520

I believe LLVM uses American English.

tschuett added inline comments.Mon, Nov 22, 9:51 AM
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
2121

Properties is unused in the function?

Meinersbur requested changes to this revision.Mon, Nov 22, 9:21 PM

Thanks for helping to complete the OpenMPIRBuilder implementation!

clang/lib/CodeGen/CGStmtOpenMP.cpp
2586–2587

[nit] Unnecessary whitespace

2587

Did you consider adding a check whether OpenMPIRBuilder actually implements all features requested before using it? See isSupportedByOpenMPIRBuilder.

2594

[style] According to the LLVM coding standard, auto is only used in specific cases.

clang/test/OpenMP/simd_codegen_irbuilder.cpp
16

Could you use regexes to match the virtual register names? The script update_cc_checks.py may help with this, see the other tests.

The naming scheme for files testing the OpenMPIRBiulder is usually irbuilder_<constructs>

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
524

Capitalizing SIMD leads to long runs of capital letters. While not an LLVM coding style rule, it is still common to title-case acronyms in identifiers if the acronym is ~4 letters or more, such as OMPSimdDirective instead of OMPSIMDDirective.

For methods transformation CanonicalLoopInfo, we don't use createXYZ naming scheme which is reserved for inserting something new. Suggestions: applySimd, vectorizeLoop, or simdizeLoop.

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
2120

The LLVM coding style still capitalizes parameters and variables.

2123

The property also applies to any memory access, e.g. memset. You could use mayWriteToMemory() and mayReadFromMemory().

2126

[serious] The access group must be unique only to the loop being accessed.

2157–2168

[serious] header and cond don't contain any memory accesses. body may consist of multiple basic blocks, of which this only applies to the first one.

This revision now requires changes to proceed.Mon, Nov 22, 9:21 PM