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

[clang] Add support for new loop attribute [[clang::code_align()]] #70762

Merged
merged 43 commits into from
Nov 20, 2023

Conversation

smanna12
Copy link
Contributor

@smanna12 smanna12 commented Oct 31, 2023

This patch adds support for new loop attribute: [[clang::code_align(N)]].
This attribute applies to a loop and specifies the byte alignment for a loop.
The attribute accepts a positive integer constant initialization expression
indicating the number of bytes for the minimum alignment boundary.
Its value must be a power of 2, between 1 and 4096 (inclusive).

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen labels Oct 31, 2023
Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

Please be sure to add a reasonable description to the patch summary so that reviewers know more about what's going on, but thank you for already having written docs (that helps give context).

The changes should come with a release note as well.

clang/include/clang/Basic/Attr.td Outdated Show resolved Hide resolved
clang/include/clang/Basic/AttrDocs.td Outdated Show resolved Hide resolved
clang/include/clang/Basic/AttrDocs.td Outdated Show resolved Hide resolved
clang/include/clang/Basic/DiagnosticSemaKinds.td Outdated Show resolved Hide resolved
clang/lib/CodeGen/CGLoopInfo.cpp Outdated Show resolved Hide resolved
clang/lib/Sema/SemaStmtAttr.cpp Outdated Show resolved Hide resolved
clang/lib/Sema/SemaStmtAttr.cpp Outdated Show resolved Hide resolved
clang/include/clang/Basic/Attr.td Outdated Show resolved Hide resolved
clang/include/clang/Basic/AttrDocs.td Outdated Show resolved Hide resolved
clang/include/clang/Basic/AttrDocs.td Outdated Show resolved Hide resolved
clang/test/Sema/code_align.c Outdated Show resolved Hide resolved
clang/test/Sema/code_align.c Outdated Show resolved Hide resolved
clang/lib/CodeGen/CGLoopInfo.cpp Show resolved Hide resolved
clang/include/clang/Basic/AttrDocs.td Outdated Show resolved Hide resolved
clang/lib/CodeGen/CGLoopInfo.cpp Outdated Show resolved Hide resolved
clang/lib/Sema/SemaStmtAttr.cpp Outdated Show resolved Hide resolved
clang/lib/Sema/SemaStmtAttr.cpp Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Nov 2, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

@smanna12
Copy link
Contributor Author

smanna12 commented Nov 2, 2023

Thank you @erichkeane and @AaronBallman for reviews and feedbacks.

Please be sure to add a reasonable description to the patch summary so that reviewers know more about what's going on, but thank you for already having written docs (that helps give context).

I have added description to the patch summary.

The changes should come with a release note as well.
Sure, I will add a release note.

I have added release note.

FreddyLeaf added a commit to FreddyLeaf/llvm-project that referenced this pull request Nov 2, 2023
… Loops.

This patch added backend consumption on a new loop metadata:
!1 = !{!"llvm.loop.align", i32 64}
which is generated from clang's new loop attribute:
[[clang::code_align()]]
clang patch: llvm#70762
Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

Thank you for the link to the LLVM side of things, that helps!

clang/lib/Sema/SemaStmtAttr.cpp Outdated Show resolved Hide resolved
clang/test/Sema/code_align.c Outdated Show resolved Hide resolved
clang/include/clang/Basic/DiagnosticSemaKinds.td Outdated Show resolved Hide resolved
clang/include/clang/Basic/DiagnosticSemaKinds.td Outdated Show resolved Hide resolved
clang/lib/CodeGen/CGLoopInfo.cpp Outdated Show resolved Hide resolved
clang/lib/Sema/SemaStmtAttr.cpp Outdated Show resolved Hide resolved
clang/lib/Sema/SemaStmtAttr.cpp Outdated Show resolved Hide resolved
@smanna12
Copy link
Contributor Author

smanna12 commented Nov 8, 2023

@AaronBallman, @erichkeane I have addressed comments in the PR, could you please revisit this PR? Thank you!

@smanna12
Copy link
Contributor Author

Thank you @erichkeane for reviews!

@smanna12 smanna12 merged commit 48ff354 into llvm:main Nov 20, 2023
4 checks passed
@smanna12 smanna12 deleted the AddCodeAlignAttr branch November 20, 2023 22:13
smanna12 added a commit to smanna12/llvm-project that referenced this pull request Nov 21, 2023
FreddyLeaf added a commit that referenced this pull request Nov 21, 2023
… Loops. (#71026)

This patch added backend consumption on a new loop metadata:
!1 = !{!"llvm.loop.align", i32 64}
which is generated from clang's new loop attribute:
[[clang::code_align()]]
clang patch: #70762
omjavaid added a commit that referenced this pull request Nov 21, 2023
erichkeane pushed a commit that referenced this pull request Nov 21, 2023
Lit test generates different outputs for usage of __int128_t in
clang-armv8-quick environment. This patch adds triple to fix the lit
failure.

```
Step 5 (ninja check 1) failure: 1 unexpected failures 38623 expected passes 71 expected failures 36752 unsupported tests (failure)
******************** TEST 'Clang :: Sema/code_align.c' FAILED ******************** Exit Code: 1

Command Output (stderr):
--
RUN: at line 1: /home/tcwg-buildbot/worker/clang-armv8-quick/stage1/bin/clang -cc1 -internal-isystem /home/tcwg-buildbot/worker/clang-armv8-quick/stage1/lib/clang/18/include -nostdsysteminc -fsyntax-only -verify=expected,c-local -x c /home/tcwg-buildbot/worker/clang-armv8-quick/llvm/clang/test/Sema/code_align.c
+ /home/tcwg-buildbot/worker/clang-armv8-quick/stage1/bin/clang -cc1 
+ -internal-isystem 
+ /home/tcwg-buildbot/worker/clang-armv8-quick/stage1/lib/clang/18/inclu
+ de -nostdsysteminc -fsyntax-only -verify=expected,c-local -x c 
+ /home/tcwg-buildbot/worker/clang-armv8-quick/llvm/clang/test/Sema/code
+ _align.c
error: 'c-local-error' diagnostics expected but not seen: 
  File /home/tcwg-buildbot/worker/clang-armv8-quick/llvm/clang/test/Sema/code_align.c Line 79 (directive at /home/tcwg-buildbot/worker/clang-armv8-quick/llvm/clang/test/Sema/code_align.c:78): 'code_align' attribute requires an integer argument which is a constant power of two between 1 and 4096 inclusive; provided argument was (__int128_t)1311768467294899680ULL << 64
  File /home/tcwg-buildbot/worker/clang-armv8-quick/llvm/clang/test/Sema/code_align.c Line 89 (directive at /home/tcwg-buildbot/worker/clang-armv8-quick/llvm/clang/test/Sema/code_align.c:88): 'code_align' attribute requires an integer argument which is a constant power of two between 1 and 4096 inclusive; provided argument was -(__int128_t)1311768467294899680ULL << 64
error: 'c-local-error' diagnostics seen but not expected: 
  File /home/tcwg-buildbot/worker/clang-armv8-quick/llvm/clang/test/Sema/code_align.c Line 79: use of undeclared identifier '__int128_t'
  File /home/tcwg-buildbot/worker/clang-armv8-quick/llvm/clang/test/Sema/code_align.c Line 89: use of undeclared identifier '__int128_t'
4 errors generated.

```
smanna12 added a commit to smanna12/llvm that referenced this pull request Dec 24, 2023
…e way we handle conflicting vs duplicate values

This patch diagnose non-identical duplicates as a 'conflicting' loop attributes
and suppress duplicate errors in cases where the two match for FPGA SYCL attributes: 'SYCLIntelMaxInterleavingAttr', 'SYCLIntelSpeculatedIterationsAttr', and
'SYCLIntelMaxReinvocationDelayAttr' to align with clang community change (Ref: llvm/llvm-project#70762).

Signed-off-by: Soumi Manna <soumi.manna@intel.com>
aelovikov-intel pushed a commit to intel/llvm that referenced this pull request Jan 10, 2024
…e way we handle conflicting vs duplicate values (#12243)

This patch diagnose non-identical duplicates as a 'conflicting' loop
attributes and suppress duplicate errors in cases where the two match
for FPGA SYCL attributes: 'SYCLIntelMaxInterleavingAttr',
'SYCLIntelSpeculatedIterationsAttr', and
'SYCLIntelMaxReinvocationDelayAttr' to align with clang community change
(Ref: llvm/llvm-project#70762).

---------

Signed-off-by: Soumi Manna <soumi.manna@intel.com>
@MaskRay
Copy link
Member

MaskRay commented Feb 6, 2024

@efriedma-quic

(came here from #80765)

smanna12 added a commit to smanna12/llvm that referenced this pull request Mar 25, 2024
…e way we handle conflicting vs duplicate values

This patch updates ExprArgument<> to use same routine CheckForDuplicateAttrs() that was added on intel#12243 to diagnose non-identical duplicates as a 'conflicting' loop attributes and suppresse duplicate errors in cases where the two match for FPGA SYCL attributes: [[intel::max_concurrency()]] and [[intel::initiation_interval()]] to align with clang community change (Ref: llvm/llvm-project#70762).

Signed-off-by: Soumi Manna <soumi.manna@intel.com>
sarnex pushed a commit to intel/llvm that referenced this pull request Mar 27, 2024
…e way we handle conflicting vs duplicate values (#13146)

This patch updates ExprArgument<> to use same routine
CheckForDuplicateAttrs() that was added on
#12243 to diagnose non-identical
duplicates as a 'conflicting' loop attributes and suppresse duplicate
errors in cases where the two match for FPGA SYCL attributes:
[[intel::max_concurrency()]] and [[intel::initiation_interval()]] to
align with clang community change (Ref:
llvm/llvm-project#70762).

---------

Signed-off-by: Soumi Manna <soumi.manna@intel.com>
smanna12 added a commit to smanna12/llvm-project that referenced this pull request Apr 2, 2024
…h loop attribute 'code_align'

llvm#70762 added support for new loop attribute [[clang::code_align()]].

This patch fixes bug for the test case below that misses diagnostic due to discontinue to loop while checking duplicate vs conflicting code_align attribute values.

[[clang::code_align(4)]]
[[clang::code_align(4)]]
[[clang::code_align(8)]]
for(int I=0; I<128; ++I) { bar(I); }
smanna12 added a commit that referenced this pull request Apr 3, 2024
…th loop attribute 'code_align' (#87372)

#70762 added support for new
loop attribute [[clang::code_align()]].

This patch fixes bugs for the test cases below that misses diagnostics due to discontinue to while loop during checking duplicate vs conflicting code_align attribute values in routine CheckForDuplicateLoopAttrs().

[[clang::code_align(4)]]
[[clang::code_align(4)]]
[[clang::code_align(8)]]
for(int I=0; I<128; ++I) { bar(I); }

[[clang::code_align(4)]]
[[clang::code_align(4)]]
[[clang::code_align(8)]]
[[clang::code_align(32)]]
for(int I=0; I<128; ++I) { bar(I); }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants