[clang] Convert second arg of __builtin_assume_aligned to ConstantExpr#161314
Merged
Conversation
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) ChangesSince the second argument must be a constant integer, we can as well convert it to a Fixes #161272 Full diff: https://github.com/llvm/llvm-project/pull/161314.diff 2 Files Affected:
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 39c3aa2243338..7b37e0b8d5430 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -5954,6 +5954,9 @@ bool Sema::BuiltinAssumeAligned(CallExpr *TheCall) {
if (Result > Sema::MaximumAlignment)
Diag(TheCall->getBeginLoc(), diag::warn_assume_aligned_too_great)
<< SecondArg->getSourceRange() << Sema::MaximumAlignment;
+
+ TheCall->setArg(1,
+ ConstantExpr::Create(Context, SecondArg, APValue(Result)));
}
if (NumArgs > 2) {
diff --git a/clang/test/SemaCXX/builtin-assume-aligned.cpp b/clang/test/SemaCXX/builtin-assume-aligned.cpp
index 48bd8414fc50a..afc11cc694705 100644
--- a/clang/test/SemaCXX/builtin-assume-aligned.cpp
+++ b/clang/test/SemaCXX/builtin-assume-aligned.cpp
@@ -47,3 +47,9 @@ constexpr void *s1 = __builtin_assume_aligned(x, 32);
constexpr void *s2 = __builtin_assume_aligned(x, 32, 5);
constexpr void *s3 = __builtin_assume_aligned(x, 32, -1);
+
+constexpr int add(int a, int b) {
+ return a+b;
+}
+constexpr void *c1 = __builtin_assume_aligned(p, add(1,1));
+constexpr void *c2 = __builtin_assume_aligned(p, add(2,1)); // expected-error {{not a power of 2}}
|
Contributor
|
Can you add a changelog entry? Thanks |
cor3ntin
approved these changes
Sep 30, 2025
tbaederr
force-pushed
the
assume-aligned
branch
from
607da57 to
b442ffb
Compare
Contributor
|
Heads up - this PR is causing us to hit the following assertion in https://github.com/google/highway. I'm currently running cvise to get a minimal reproducer. |
Contributor
The repro file is 2.5M, so cvise will take forever. This will take a while, so I apologize in advance. |
Contributor
|
Reduced repro: constexpr long kAlignment = 128;
long AllocateAlignedBytes_payload;
void AllocateAlignedBytes() {
__builtin_assume_aligned(
reinterpret_cast<void *>(AllocateAlignedBytes_payload), kAlignment);
} |
Contributor
DKLoehr
added a commit
to DKLoehr/llvm-project
that referenced
this pull request
Oct 2, 2025
…stantExpr (llvm#161314)" This reverts commit 8f77621.
Contributor
|
Since this is a crash and it's blocking us from finding any other issues at chromium, let's revert this for now. |
Contributor
That sounds unfortunate but reasonable. Thanks for handling the revert. |
Contributor
|
@Fznamznon : Your patch 9ad72df seems at least closely related here: in CGExprScalar.cpp in VisitConstantExpr. This patch is unfortunately just revealing a bug there, where the type of the constant expression is a L value constant but isn't getting the 'pointer' type for the It isn't clear to me what SHOULD happen, but that at least seems wrong. |
DKLoehr
added a commit
that referenced
this pull request
Oct 2, 2025
…stantExpr (#161314)" (#161719) This reverts commit 8f77621 (#161314). Reason: Causing crashes when building https://github.com/google/highway. See the original PR for details.
mahesh-attarde
pushed a commit
to mahesh-attarde/llvm-project
that referenced
this pull request
Oct 3, 2025
llvm#161314) Since the second argument must be a constant integer, we can as well convert it to a `ConstantExpr` in Sema. Fixes llvm#161272
mahesh-attarde
pushed a commit
to mahesh-attarde/llvm-project
that referenced
this pull request
Oct 3, 2025
…stantExpr (llvm#161314)" (llvm#161719) This reverts commit 8f77621 (llvm#161314). Reason: Causing crashes when building https://github.com/google/highway. See the original PR for details.
Contributor
Author
|
It seems like we're doing the load in |
tbaederr
added a commit
to tbaederr/llvm-project
that referenced
this pull request
Oct 4, 2025
…nstantExpr (llvm#161314)" (llvm#161719) This reverts commit f1650cf.
tbaederr
added a commit
that referenced
this pull request
Oct 5, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Since the second argument must be a constant integer, we can as well convert it to a
ConstantExprin Sema.Fixes #161272