-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[CIR] Handle overlapping values in constant init expressions #164508
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
Conversation
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clangir Author: Andy Kaylor (andykaylor) ChangesThis adds handling for generating constant initializers in the case where a value we are emitting overlaps with a previous constant, such as can happen when initializing a structure with bitfields. Full diff: https://github.com/llvm/llvm-project/pull/164508.diff 2 Files Affected:
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp b/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp
index 65e6a3915f241..13bcd54ece291 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp
@@ -179,8 +179,23 @@ bool ConstantAggregateBuilder::add(mlir::TypedAttr typedAttr, CharUnits offset,
}
// Uncommon case: constant overlaps what we've already created.
- cgm.errorNYI("overlapping constants");
- return false;
+ std::optional<size_t> firstElemToReplace = splitAt(offset);
+ if (!firstElemToReplace)
+ return false;
+
+ CharUnits cSize = getSize(typedAttr);
+ std::optional<size_t> lastElemToReplace = splitAt(offset + cSize);
+ if (!lastElemToReplace)
+ return false;
+
+ assert((firstElemToReplace == lastElemToReplace || allowOverwrite) &&
+ "unexpectedly overwriting field");
+
+ Element newElt(typedAttr, offset);
+ replace(elements, *firstElemToReplace, *lastElemToReplace, {newElt});
+ size = std::max(size, offset + cSize);
+ naturalLayout = false;
+ return true;
}
bool ConstantAggregateBuilder::addBits(llvm::APInt bits, uint64_t offsetInBits,
diff --git a/clang/test/CIR/CodeGen/struct-init.cpp b/clang/test/CIR/CodeGen/struct-init.cpp
index 2887e6f404ffc..ff1b06308e74e 100644
--- a/clang/test/CIR/CodeGen/struct-init.cpp
+++ b/clang/test/CIR/CodeGen/struct-init.cpp
@@ -5,6 +5,21 @@
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll
// RUN: FileCheck --check-prefix=OGCG --input-file=%t.ll %s
+struct O {
+ unsigned int a:4;
+ unsigned int b:14;
+ unsigned int c:14;
+};
+
+O overlapping_init = { 3, 2, 1 };
+
+// This is unintuitive. The bitfields are initialized using a struct of constants
+// that maps to the bitfields but splits the value into bytes.
+
+// CIR: cir.global external @overlapping_init = #cir.const_record<{#cir.int<35> : !u8i, #cir.int<0> : !u8i, #cir.int<4> : !u8i, #cir.int<0> : !u8i}> : !rec_anon_struct
+// LLVM: @overlapping_init = global { i8, i8, i8, i8 } { i8 35, i8 0, i8 4, i8 0 }
+// OGCG: @overlapping_init = global { i8, i8, i8, i8 } { i8 35, i8 0, i8 4, i8 0 }
+
struct S {
int a, b, c;
};
|
AmrDeveloper
left a comment
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.
LGTM
| // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll | ||
| // RUN: FileCheck --check-prefix=OGCG --input-file=%t.ll %s | ||
|
|
||
| struct O { |
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.
Would love for this to have a better name :) Just a few characters even. Otherwise O overlapping_init looks a LOT like 1 word/was jarring.
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.
Yeah, I didn't like that either and I'm the one who wrote it! I'm not sure why I left it that way.
This adds handling for generating constant initializers in the case where a value we are emitting overlaps with a previous constant, such as can happen when initializing a structure with bitfields.
6dff586 to
f4a6bb7
Compare
…4508) This adds handling for generating constant initializers in the case where a value we are emitting overlaps with a previous constant, such as can happen when initializing a structure with bitfields.
…4508) This adds handling for generating constant initializers in the case where a value we are emitting overlaps with a previous constant, such as can happen when initializing a structure with bitfields.
…4508) This adds handling for generating constant initializers in the case where a value we are emitting overlaps with a previous constant, such as can happen when initializing a structure with bitfields.
…4508) This adds handling for generating constant initializers in the case where a value we are emitting overlaps with a previous constant, such as can happen when initializing a structure with bitfields.
This adds handling for generating constant initializers in the case where a value we are emitting overlaps with a previous constant, such as can happen when initializing a structure with bitfields.