https://github.com/dwblaikie approved this pull request.
https://github.com/llvm/llvm-project/pull/130255
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
dwblaikie wrote:
yeah, can't say I have any recollection of why function declaration would be in
a "retained types" list - probably incidental/not especially intentional (we
didn't make declarations for functions, except member functions, until
"recently" when call site debug info was added).
https://github.com/dwblaikie commented:
Could you remove the LLVM tests? They don't add coverage since tnhis isn't an
LLVM feature - as far as LLVM is concerned, this is another static member
variable like any other.
Why do some of the test cases use distinct input files/headers? I'd have
tho
https://github.com/dwblaikie commented:
We can skip the llvm testing here, I think - there are no LLVM changes to test.
(as far as LLVM is concerned, this is just another static member variable)
Got measurements on debug info size growth or any other metrics we should be
considering?
https://
dwblaikie wrote:
> > My intent (haven't checked the patch) is that it'd be modeled as a static
> > member variable - so there'd be a declaration in the class, but a
> > definition DIE outside the class that'd be indexed by gdb OK, I'd have
> > thought? (it'd go in .debug_names, and gdb_index,
dwblaikie wrote:
(I'm open to being overruled by other folks/perspectives if the straight up
global variable is preferred - other folks who are in support of that?)
https://github.com/llvm/llvm-project/pull/130255
___
cfe-commits mailing list
cfe-com
dwblaikie wrote:
Ah, yeah, I see the example from
https://github.com/llvm/llvm-project/pull/130255#issuecomment-2866460040 isn't
consistent with what I had in mind (in the example there the member DIE is a
definition - I don't think many consumers will be ready to handle that, since
it's not
dwblaikie wrote:
> > Uploaded a patch that eliminates the global variable and it moves the
> > vtable information into the static member; in that way, a consumer always
> > will have access to the vtable information, just by having the object
> > instance or the object definition.
>
> IIUC th
https://github.com/dwblaikie approved this pull request.
https://github.com/llvm/llvm-project/pull/137818
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -169,6 +169,47 @@ See the discussion in the section about
:ref:`merging locations` for examples of when the rule for
dropping locations applies.
+.. _NewInstLocations:
+
+Setting locations for new instructions
+--
+
+Whenever a new instru
@@ -169,6 +169,47 @@ See the discussion in the section about
:ref:`merging locations` for examples of when the rule for
dropping locations applies.
+.. _NewInstLocations:
+
+Setting locations for new instructions
+--
+
+Whenever a new instru
https://github.com/dwblaikie approved this pull request.
https://github.com/llvm/llvm-project/pull/134100
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
dwblaikie wrote:
> > Removing the vtable global variable and moving the "location info" into the
> > static within the class, will work for the SCE debugger.
>
> I was thinking about this last night and wondering if the vtable will appear
> as a class member even if the class is local to a fun
dwblaikie wrote:
> FYI: There is already VTable support in our lldb::SBValue class and it is
> part of the public API in LLDB and doesn't require any of this:
> ...
> Doesn't require any debug info.
Does this/can this be used to determine the type of an object that points to
that vtable, thoug
dwblaikie wrote:
> The link from the class to the specific vtable even seems mildly incorrect,
> in that during object construction the vtable will go through several
> different values, not just one.
Not sure I follow this - the object is only of the type, in some sense, when it
is pointing
dwblaikie wrote:
> It would be better if we could go from the vtable pointer straight to the
> right type DIE, but I don't think we can do that without inventing a new sort
> of an accelerator table (which I guess we don't have an appetite for).
yeah, data address lookup (as opposed to code ad
dwblaikie wrote:
> Could you please take a look at the attached DWARF and let me know your
> concerns with the implementation?
Sure - I see the effect, but I still think it's probably not worthwhile due to
the limited capacity, inconsistency (due to that limited expressiveness)
between direct
dwblaikie wrote:
> Are you suggesting that for the implementation to be considered as complete,
> both v1 and v2 should have the same type information? I.e "v1" type should
> point to 0x48 instead of 0x6d? As per my understanding based on the DWARF
> output below, the type for "trait::type"(0x
https://github.com/dwblaikie approved this pull request.
Sounds good to me - thanks for writing it up! (could wait for other folks to
chime in too, but seems pretty harmless to launch/iterate/etc)
https://github.com/llvm/llvm-project/pull/130803
___
c
dwblaikie wrote:
> I am not sure that I understand your concern completely. Consider the
> following DWARF output based on my implementation. How would you say "v1"
> should be represented ideally?
> ```
> 0x0042: DW_TAG_structure_type
> DW_AT_calling_convention (DW_CC_pass_by_value)
> DW_A
dwblaikie wrote:
> > Mostly I worry it won't be terribly complete, because it can't work through
> > situations, like this:
> > ```
> > template
> > struct trait {
> > using type = T;
> > };
> > template
> > struct other {
> > trait::type v1;
> > T v2;
> > };
> > ```
> > In this case, v2
dwblaikie wrote:
I wouldn't mind a few more details here on the motivation.
> This new symbol will allow a debugger to easily associate classes with the
> physical location of their VTables using only the DWARF information.
What sort of features are you picturing building with this?
The DWAR
dwblaikie wrote:
> For template classes with type parameters, the information of which member
> variables are of a "templated type" is lost. The debugger cannot
> differentiate between "templated" types and "hardcoded" types. This is
> because both types have the same debug information, so the
dwblaikie wrote:
> > > I don't quite follow the motivation for this, can you expand on this in
> > > the description please? Right now this seems unnecessary for the changes
> > > I can see in MLIR for example.
> >
> >
> > The linked bug seems to explain it, I think? It seems to be the usual
dwblaikie wrote:
> I don't quite follow the motivation for this, can you expand on this in the
> description please? Right now this seems unnecessary for the changes I can
> see in MLIR for example.
The linked bug seems to explain it, I think? It seems to be the usual "if
something isn't/does
https://github.com/dwblaikie approved this pull request.
https://github.com/llvm/llvm-project/pull/125979
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
dwblaikie wrote:
Could you add a test case - check in clang/test to see if other tests for the
diagnostic text in the original bug, and add a test case for that nearby (maybe
the same file the diagnostic is already tested in)?
https://github.com/llvm/llvm-project/pull/125370
__
dwblaikie wrote:
Is my thing about the ctor understanding correct, and it's that the ctor exists
theoretically/abstractly, but not in the AST or in the generated IR/DWARF?
Could it be added/would that be sufficient?
But, yeah, probably fine to just add the attribute, since that's the real
fea
https://github.com/dwblaikie closed
https://github.com/llvm/llvm-project/pull/124643
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
dwblaikie wrote:
Do you have commit access, or need someone to merge this on your behalf?
https://github.com/llvm/llvm-project/pull/124643
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi
dwblaikie wrote:
(hmm, can't use the foundation library on godbolt/compiler explorer)
Can you show a small example of the code you're interested in and the DWARF it
produces? The documentation I can find seems to suggest that the extensible
enums have ctors that take the underlying integer type
https://github.com/dwblaikie approved this pull request.
Looks great - thanks!
https://github.com/llvm/llvm-project/pull/124643
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
dwblaikie wrote:
yeah, I was going to say - thought we carved out part (the debug info part,
maybe some other things - ORC?) of the C API as unstable, so I'm OK continuing
with that (lack of) guarantee...
https://github.com/llvm/llvm-project/pull/122928
dwblaikie wrote:
> > For instance, the StartFrame/EndFrame feature, if I'm understanding it
> > correctly, seems like maybe it could be skipped if we're emitting DWARF (&
> > instead the skip attribute could be placed on all the calls between
> > StartFrame and EndFrame?).
>
> does [#123683
https://github.com/dwblaikie approved this pull request.
Generally looks fine to me - given the central nature of the data structure,
wouldn't mind a second/more authoritative set of eyes if someone has time.
You mention the performance tradeoff of pascal strings v null terminated ones
doesn't
@@ -51,6 +53,14 @@ class StringTable {
constexpr Offset() = default;
constexpr Offset(unsigned Value) : Value(Value) {}
+constexpr bool operator==(const Offset &RHS) const {
dwblaikie wrote:
Minor convention, but prefer non-member (can still be de
https://github.com/dwblaikie edited
https://github.com/llvm/llvm-project/pull/123302
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/dwblaikie approved this pull request.
Looks OK to me.
https://github.com/llvm/llvm-project/pull/122928
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Author: David Blaikie
Date: 2025-01-15T23:20:40Z
New Revision: 504dd577675e8c85cdc8525990a7c8b517a38a89
URL:
https://github.com/llvm/llvm-project/commit/504dd577675e8c85cdc8525990a7c8b517a38a89
DIFF:
https://github.com/llvm/llvm-project/commit/504dd577675e8c85cdc8525990a7c8b517a38a89.diff
LOG:
dwblaikie wrote:
> > Just that the last bit of performance
>
> Maybe we're talking about different situations; I may have missed some
> context in the discussion of this PR. I didn't see this as being about the
> last bit of performance. It sounded like this was going to knowingly
> introduce
dwblaikie wrote:
> > > Anyway, my main point was that there are people who care about
> > > performance on Windows, so please don't treat it as a second-class
> > > citizen.
> >
> >
> > Yeah, I don't want to treat folks on Windows as second class citizens by
> > any means - but I wouldn't mi
dwblaikie wrote:
> Anyway, my main point was that there are people who care about performance on
> Windows, so please don't treat it as a second-class citizen.
Yeah, I don't want to treat folks on Windows as second class citizens by any
means - but I wouldn't mind/hope we can treat folks who c
dwblaikie wrote:
> > Do we care about performance on MSVC ?
>
> Just responding to this point (although I accept it's moot in the current
> context): yes we care about performance when building with MSVC - our
> downstream toolchain is hosted on Windows. I'm sure we're not the only
> Windows
@@ -803,10 +804,16 @@ bool ConstStructBuilder::Build(const InitListExpr *ILE,
bool AllowOverwrite) {
if (!AppendField(Field, Layout.getFieldOffset(FieldNo), EltInit,
AllowOverwrite))
return false;
- // After emitting a non-empty field
https://github.com/dwblaikie commented:
Thanks! Looks plausible to me, would be great to get @Michael137's take on it,
since he's been looking at this sort of thing a fair bit lately.
https://github.com/llvm/llvm-project/pull/122197
___
cfe-commits ma
https://github.com/dwblaikie edited
https://github.com/llvm/llvm-project/pull/122197
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/dwblaikie approved this pull request.
https://github.com/llvm/llvm-project/pull/11
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/dwblaikie approved this pull request.
https://github.com/llvm/llvm-project/pull/119445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
dwblaikie wrote:
yeah, I worry this feels all a bit vague (that some things "suggest" other
things, that root problems "seem to be" (& some fairly broad descriptions of
what they seem to be) - not to nitpick your language, and I appreciate the
clarity of the uncertainty, to be sure - but the u
dwblaikie wrote:
any chance you could A/B test this on a bootstrap of clang, for instance? To
help validate that this really is NFC/dead code?
https://github.com/llvm/llvm-project/pull/119445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
htt
dwblaikie wrote:
> > Maybe it's too big/complex a problem to try to think about, and incremental
> > development should overrule here - but may be worth thinking about?
>
> Urgh. I'll admit that I hadn't thought that far ahead, I just wanted to get
> the patch-series going again now that the r
@@ -2021,28 +2021,10 @@ llvm::DISubroutineType
*CGDebugInfo::getOrCreateInstanceMethodType(
// ThisPtr may be null if the member function has an explicit 'this'
// parameter.
if (!ThisPtr.isNull()) {
-const CXXRecordDecl *RD = ThisPtr->getPointeeCXXRecordDecl();
-
dwblaikie wrote:
No idea if this sufficiently overlaps with, or is orthogonal to the following,
but:
Currently we don't scope function-local types into the appropriate scope inside
a function. It seems like associating types with their declaration may make
this harder to address - how would w
https://github.com/dwblaikie approved this pull request.
This looks fine to me - though given the broad impact (to every target in
clang) maybe @AaronBallman wants to weigh in.
If you wanted to front-load the things like `getRecord(ID).Attributes` ->
`getAttributesString(ID)` those things coul
@@ -1834,6 +1834,14 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions
&Opts, ArgList &Args,
Opts.setInlining(CodeGenOptions::NormalInlining);
}
+ // If we have specified -Og and have not explicitly set
-fno-extend-lifetimes,
+ // then default to -fextend-li
@@ -1353,6 +1353,19 @@ void CodeGenFunction::EmitLifetimeEnd(llvm::Value *Size,
llvm::Value *Addr) {
C->setDoesNotThrow();
}
+void CodeGenFunction::EmitFakeUse(Address Addr) {
+ // We do not emit a fake use if we want to apply optnone to this function,
+ // even if we mig
@@ -1353,6 +1353,19 @@ void CodeGenFunction::EmitLifetimeEnd(llvm::Value *Size,
llvm::Value *Addr) {
C->setDoesNotThrow();
}
+void CodeGenFunction::EmitFakeUse(Address Addr) {
+ // We do not emit a fake use if we want to apply optnone to this function,
+ // even if we mig
dwblaikie wrote:
(please don't merge PRs that haven't been approved, that's against LLVM
practices/policies ( https://llvm.org/docs/CodeReview.html#code-review-workflow
), unless they weren't intended for pre-commit review in the first place (if
they were only meant for presubmit checks))
htt
dwblaikie wrote:
Yeah, I think given how wide the change is and the speculative nature of the
harm - probably best to abandon this for now. We can pick it up again if more
folks speak up/more evidence is presented that it's problematic.
https://github.com/llvm/llvm-project/pull/110692
@@ -211,6 +211,16 @@ New Compiler Flags
only for thread-local variables, and none (which corresponds to the
existing ``-fno-c++-static-destructors`` flag) skips all static
destructors registration.
+- The ``-fextend-lifetimes`` and ``-fextend-this-ptr`` flags have been ad
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 %s -emit-llvm -O2 -fextend-lifetimes -o - | FileCheck
--check-prefixes=CHECK-ALL,CHECK-O2 %s
+// RUN: %clang_cc1 %s -emit-llvm -O0 -fextend-lifetimes -o - | FileCheck
--check-prefixes=CHECK-ALL,CHECK-O0 %s
+
+// Checks that we emit the functi
@@ -2217,6 +2217,11 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions
&Opts, ArgList &Args,
Args.getAllArgValues(OPT_fsanitize_trap_EQ), Diags,
Opts.SanitizeTrap);
+ Opts.ExtendThisPtr =
+ Opts.OptimizationLevel > 0
dwblaikie wrote:
Thanks @jimingham - yeah, that makes sense. My original argument for being OK
with the de-canonicalization of typedefs was that they aren't load bearing
anyway. But if lldb puts load on them, that assumption doesn't hold.
Yeah, could check for angles or template parameter DIEs
dwblaikie wrote:
Not quite sure how we get to caching and template specializations and pretty
printers.
If we're still producing the typedef-style DWARF for these alias template
specializations - perhaps lldb could not cache pretty printers for typedefs? (I
guess the pretty printers shouldn't
https://github.com/dwblaikie approved this pull request.
Fair enough
https://github.com/llvm/llvm-project/pull/108300
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -gno-column-info
-triple=x86_64-pc-linux -emit-llvm %s -o - | FileCheck %s
+
+// The important thing is that the compare and the conditional branch have
+// locs with the same scope (the lexical block for the 'if'). B
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -gno-column-info
-triple=x86_64-pc-linux -emit-llvm %s -o - | FileCheck %s
+
+// The important thing is that the compare and the conditional branch have
+// locs with the same scope (the lexical block for the 'if'). B
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -gno-column-info
-triple=x86_64-pc-linux -emit-llvm %s -o - | FileCheck %s
+
+// The important thing is that the compare and the conditional branch have
+// locs with the same scope (the lexical block for the 'if'). B
https://github.com/dwblaikie approved this pull request.
https://github.com/llvm/llvm-project/pull/106033
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
dwblaikie wrote:
> This make sense to me, @dwblaikie wdyt?
Seems OK to me
https://github.com/llvm/llvm-project/pull/106033
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
dwblaikie wrote:
What static analysis tool flagged this? Any idea why this particular null
dereference, when LLVM/Clang have loads of assumed-non-null pointers that are
dereferenced in a great many places?
https://github.com/llvm/llvm-project/pull/105556
___
dwblaikie wrote:
> Looking forward to trying this out!
>
> Should the new flag have some docs and maybe be mentioned in the release
> notes, or do we think it's not ready for prime time yet for some reason?
I'm /pretty/ neutral on that - it's got pretty clear behavior, etc (& in fact
some cla
dwblaikie wrote:
> > The motivation is as usual IWYU and similar refactoring - to reduce build
> > time and probablility of non-related source(s) recompile.
>
> I'm confused: as far as I know IWYU achieves the opposite of what you're
> describing actually: it adds more includes than strictly n
dwblaikie wrote:
Is there existing test coverage (like, if you add an assertion, instead of an
if/return - do any tests hit the assertion)? or could you add some?
https://github.com/llvm/llvm-project/pull/97933
___
cfe-commits mailing list
cfe-commits
https://github.com/dwblaikie approved this pull request.
LGTM
https://github.com/llvm/llvm-project/pull/100767
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Author: David Blaikie
Date: 2024-07-25T21:52:14Z
New Revision: 8dd574236ccaa0a183278396cfec3068b832651a
URL:
https://github.com/llvm/llvm-project/commit/8dd574236ccaa0a183278396cfec3068b832651a
DIFF:
https://github.com/llvm/llvm-project/commit/8dd574236ccaa0a183278396cfec3068b832651a.diff
LOG:
@@ -98,3 +98,6 @@ enum E8 { A8 = -128, B8 = 127 } x8;
// CHECK-NOT: DIFlagEnumClass
// CHECK: !DIEnumerator(name: "A8", value: -128)
+// Forward declaration of an enum class.
+enum class Color : int;
+// CHECK-NOT: !DICompositeType(tag: DW_TAG_enumeration_type, name: "Color"
-
dwblaikie wrote:
also, if you're sending a pull request just because it's convenient (via the
web UI "revert" button) or to run presubmits but not for review, please include
the skip-presubmit-approval label so it's clear something isn't getting
committed without needed review
https://github.
dwblaikie wrote:
(if you don't need a code review for a change, you can commit it directly
without a pull request - if you're making a PR not for code review but for
presubmit checks, please label to `skip-precommit-approval` to clarify that you
aren't waiting for approval (and that the change
dwblaikie wrote:
> > That's a pretty substantial policy change to propose, and this probably
> > isn't the place to propose/discuss it. If that's your intent, probably best
> > to take that up on discord.
>
> I am not proposing a policy change. I believe the current policy is aimed at
> givin
dwblaikie wrote:
> Thanks @smanna12. I think this looks ok; returning null here does appear to
> be consistent with other overloads of `CreateTypeDefinition` for `RecordType`
> and `ObjCInterfaceType`. I agree with @Michael137 that it would be nice to
> have an example that fails the added con
dwblaikie wrote:
> Also, I think we presently have sufficient review bandwidth in clang proper,
> even including the area of modules, so that any non-trivial change can be
> reviewed, and I also propose that we abstain from trying to directly commit
> those.
That's a pretty substantial policy
dwblaikie wrote:
> I think the appropriate tag for such commits would be NFCI, and should still
> require PR and review.
Clarifying a couple of things here
1) I tend to agree the patch might've been a bit subtle and maybe split up into
smaller, independent parts (usual rules: [be an isolated c
@@ -185,6 +203,18 @@ CALL_AO(PackedMembers)
// CHECK: call void @llvm.memcpy.p0.p0.i64({{.*}} align 1 {{.*}} align 1
{{.*}}i64 16, i1 {{.*}})
// CHECK: ret ptr
+// WithEmptyField copy-assignment:
+// CHECK-LABEL: define linkonce_odr nonnull align {{[0-9]+}}
dereferenceable({
dwblaikie wrote:
> It looks like the presence of `static` on template variable specializations
> makes difference in the namespace context: https://gcc.godbolt.org/z/WGsreqbz8
>
> Specifically, the specializations not marked `static` result in an exported
> variable. Thus, we have seemingly va
@@ -0,0 +1,25 @@
+// RUN: not %clang -### -c --target=x86_64 -Wa,--crel %s 2>&1 | FileCheck %s
--check-prefix=NOEXP
+
+// NOEXP: error: -Wa,--allow-experimental-crel must be specified to use
-Wa,--crel. CREL is experimental and takes a non-standard section type code
+
+// RUN: %
https://github.com/dwblaikie approved this pull request.
https://github.com/llvm/llvm-project/pull/97176
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/dwblaikie edited
https://github.com/llvm/llvm-project/pull/97176
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/dwblaikie closed
https://github.com/llvm/llvm-project/pull/96699
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
dwblaikie wrote:
> Minor nit: Probably don't need to explicitly define a diagnostic group, since
> it's only ever used for this diagnostic.
Not sure what the conventions are - but I rather like the normalization of
having the explicit group, better chance of finding the group/reusing it maybe?
dwblaikie wrote:
@cor3ntin ping on this?
https://github.com/llvm/llvm-project/pull/93430
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
dwblaikie wrote:
Oh, this code was touched really recently in
66df6141659375e738d9b9b74bf79b2317576042 (@augusto2112 ) (see notes in previous
comments about how this code should be generalized)
https://github.com/llvm/llvm-project/pull/93809
___
cfe-
dwblaikie wrote:
Here's the smallest patch that would put explicit alignment on any packed
structure:
```
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp
b/clang/lib/CodeGen/CGDebugInfo.cpp
index a072475ba770..bbb13ddd593b 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CG
dwblaikie wrote:
The other effects of `packed` are encoded (the changes to the member offsets)
but that's not the only effect, and we shouldn't use that effect (which isn't
guaranteed to be observable - as in the case of "packed struct with a single
member/that just happens to not have padding
dwblaikie wrote:
oh, slight misquote above - `aligned(1)` is a no-op, as that attribute can't
reduce the alignment...
But I think my argument still stands, roughly as - if increases in alignment
are explicitly specified, decreases in alignment should be too.
https://github.com/llvm/llvm-proj
dwblaikie wrote:
Ah, I think this right solution to /this/ case is that the compiler should be
emitting the alignment for the structure?
Like there's no way for the debugger to know that this structure is
underaligned at the moment:
```
struct __attribute__((packed)) t1 {
int i;
};
```
Which
dwblaikie wrote:
> Should we add a test for this flag? Something around existing tests in
> clang/test/CXX/temp/temp.spec/temp.expl.spec/p2-20.cpp and
> clang/test/CXX/dcl.dcl/dcl.spec/dcl.stc/p1.cpp.
Oh, right, I did have a new test I meant to add, but that's a better place for
it - so I've
https://github.com/dwblaikie updated
https://github.com/llvm/llvm-project/pull/96699
>From a539afc7b81502ffcab7028bfe8266b8e32951d9 Mon Sep 17 00:00:00 2001
From: David Blaikie
Date: Tue, 25 Jun 2024 21:02:50 +
Subject: [PATCH 1/2] Clang: Add warning flag for storage class specifiers on
ex
@@ -86,6 +86,8 @@ DYNAMIC_TAG(RELRSZ, 35) // Size of Relr relocation table.
DYNAMIC_TAG(RELR, 36)// Address of relocation table (Relr entries).
DYNAMIC_TAG(RELRENT, 37) // Size of a Relr relocation entry.
+DYNAMIC_TAG(CREL, 38) // CREL relocation table
+
--
dwblaikie wrote:
Sent a patch to add a warning flag for the warning this patch uses:
https://github.com/llvm/llvm-project/pull/96699
With that, we could disable the warning during the compiler migration,
decoupling compiler migration from code cleanup.
https://github.com/llvm/llvm-project/pul
1 - 100 of 1184 matches
Mail list logo
