efriedma added a comment.
Starting multiple threads on the LLVM mailing lists with the same message is
spam. Please don't do that.
If you think you've run into a bug, file a bug report at bugs.llvm.org. For
general questions, send an email to llvm-dev.
Repository:
rC Clang
https://review
stefson added a comment.
hey there, I've run into problems with stripping static-libs on arm when using
llvm/clang-7. Could you imagine this patch being at fault?
strip: armv7a-unknown-linux-gnueabihf-strip --strip-unneeded -R .comment -R
.GCC.command.line -R .note.gnu.gold-version
lib/l
This revision was automatically updated to reflect the committed changes.
Closed by commit rC338279: [ARM, AArch64]: Use unadjusted alignment when
passing composites as arguments (authored by chill, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D46013
Files:
include/clang/AS
efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D46013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/c
chill marked an inline comment as done.
chill added a comment.
Thanks for pointing this!
https://reviews.llvm.org/D46013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
chill updated this revision to Diff 157662.
chill added a comment.
Fixed to properly examine the canonical type when getting it's unadjusted
alignment.
https://reviews.llvm.org/D46013
Files:
include/clang/AST/ASTContext.h
include/clang/AST/RecordLayout.h
lib/AST/ASTContext.cpp
lib/AST/
efriedma added inline comments.
Comment at: lib/AST/ASTContext.cpp:2088
+ default:
+UnadjustedAlign = getTypeAlign(T);
+ }
This "default" isn't right; there are a lot of non-canonical types which need
to be handled here (which getTypeAlign handles, but thi
chill added a comment.
Ping?
https://reviews.llvm.org/D46013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
chill added a comment.
Ping?
https://reviews.llvm.org/D46013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
chill added a comment.
Ping?
https://reviews.llvm.org/D46013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
chill added a comment.
Ping?
https://reviews.llvm.org/D46013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
chill updated this revision to Diff 152679.
chill added a comment.
Update: use "unadjusted alignment" instead of "natural alignment", rename
things accordingly.
https://reviews.llvm.org/D46013
Files:
include/clang/AST/ASTContext.h
include/clang/AST/RecordLayout.h
lib/AST/ASTContext.cpp
t.p.northover added a comment.
> I'd rather not put target names in API functions. The meaning of that field
> is pretty target independent ("alignment of type, before alignment
> adjustments")
Except that rule only applies to aggregates. Scalar types get their alignment
adjusted anyway I beli
chill added inline comments.
Comment at: lib/CodeGen/TargetInfo.cpp:5055
+ Alignment = getContext().getTypeNaturalAlign(Ty);
+ Alignment = std::min(std::max(Alignment, 64u), 128u);
+} else {
t.p.northover wrote:
> I think the max/min logic is more c
chill added a comment.
In https://reviews.llvm.org/D46013#1140336, @t.p.northover wrote:
> I'm fine with the ABI changes, but I'm not very convinced by the
> "NaturalAlignment" name.
I'd rather not put target names in API functions. The meaning of that field is
pretty target
independent ("ali
t.p.northover added a comment.
I'm fine with the ABI changes, but I'm not very convinced by the
"NaturalAlignment" name.
Far from being a privileged alignment kind, it seems to take account of a
pretty arbitrary set of modifiers. In fact, I can't help wondering if what's
really happened is tha
chill added a comment.
Ping?
https://reviews.llvm.org/D46013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
chill added a comment.
Ping?
https://reviews.llvm.org/D46013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rjmccall added a reviewer: t.p.northover.
rjmccall added a comment.
Okay, as a code change this seems more reasonable to me. I haven't really
thought through the ABI-compatibility issues, though. CC'ing Tim.
https://reviews.llvm.org/D46013
___
cf
chill updated this revision to Diff 150148.
chill added a comment.
Update: refactor a bit to not impose size overhead on targets, which don't use
natural alignment.
https://reviews.llvm.org/D46013
Files:
include/clang/AST/ASTContext.h
include/clang/AST/RecordLayout.h
lib/AST/ASTContext.c
rjmccall added a comment.
In https://reviews.llvm.org/D46013#1119513, @efriedma wrote:
> > I'm not sure it's appropriate to impose this as an overhead on all targets.
>
> You mean the overhead of increasing the size of TypeInfo? That's fair.
Yeah. It seems like something that could be pretty
efriedma added a comment.
> I'm not sure it's appropriate to impose this as an overhead on all targets.
You mean the overhead of increasing the size of TypeInfo? That's fair.
https://reviews.llvm.org/D46013
___
cfe-commits mailing list
cfe-commits
rjmccall added a comment.
I'm not sure it's appropriate to impose this as an overhead on all targets.
https://reviews.llvm.org/D46013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
chill marked 2 inline comments as done.
chill added a comment.
In https://reviews.llvm.org/D46013#1118014, @efriedma wrote:
> I'm not sure Apple will want to mess with their ABI like this... adding some
> reviewers.
Fair enough, I'd rather not touch it :)
https://reviews.llvm.org/D46013
_
chill updated this revision to Diff 149536.
chill added a comment.
Update:
- fix only APCS, don't touch other ABIs
- misc other
https://reviews.llvm.org/D46013
Files:
include/clang/AST/ASTContext.h
include/clang/AST/RecordLayout.h
lib/AST/ASTContext.cpp
lib/AST/RecordLayout.cpp
lib/A
efriedma added reviewers: aemerson, rjmccall.
efriedma added a comment.
I'm not sure Apple will want to mess with their ABI like this... adding some
reviewers.
Otherwise LGTM.
Comment at: lib/CodeGen/TargetInfo.cpp:5790
// than ABI alignment.
- uint64_t ABIAlign = 4;
- u
chill updated this revision to Diff 149252.
chill added a comment.
Update:
- similar changes needed for AArch64
- added/updated tests
https://reviews.llvm.org/D46013
Files:
include/clang/AST/ASTContext.h
include/clang/AST/RecordLayout.h
lib/AST/ASTContext.cpp
lib/AST/RecordLayout.cpp
chill added a comment.
In https://reviews.llvm.org/D46013#1084440, @efriedma wrote:
> I'd like to see some tests for __attribute((packed)).
Thanks, indeed it does not work correctly on packed structures. Back to the
drawing board ...
Repository:
rC Clang
https://reviews.llvm.org/D46013
efriedma added a comment.
I'd like to see some tests for __attribute((packed)).
Comment at: lib/CodeGen/TargetInfo.cpp:5787
+ } else {
+ TyAlign = getContext().getTypeAlign(Ty) ;
+ }
Whitespace.
Repository:
rC Clang
https://reviews.llvm.org/D46013
chill added a comment.
Ping?
Repository:
rC Clang
https://reviews.llvm.org/D46013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
chill created this revision.
chill added reviewers: john.brawn, olista01, eli.friedman, rengolin.
Herald added a reviewer: javed.absar.
Herald added subscribers: chrib, kristof.beyls.
The "Procedure Call Procedure Call Standard for the ARM® Architecture"
(https://static.docs.arm.com/ihi0042/f/IH
31 matches
Mail list logo
