[PATCH] D78785: Fix x86/x86_64 calling convention for _ExtInt

rjmccall added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78785/new/ https://reviews.llvm.org/D78785 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D78785: Fix x86/x86_64 calling convention for _ExtInt

erichkeane added a comment. Alright then, I've disabled by default(911add149af563a5a61458de0dd730e3f5348623 ). I'll enable 1 at a time as I fix the calling conventions in D79118 .

[PATCH] D78785: Fix x86/x86_64 calling convention for _ExtInt

erichkeane added a comment. See my audit/WIP on calling conventions here: https://reviews.llvm.org/D79118 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78785/new/ https://reviews.llvm.org/D78785 ___

[PATCH] D78785: Fix x86/x86_64 calling convention for _ExtInt

rjmccall added a comment. In D78785#2010738 , @erichkeane wrote: > In D78785#2010654 , @rjmccall wrote: > > > Every target does something similar, they just all do it in different ways > > because they're mostly

[PATCH] D78785: Fix x86/x86_64 calling convention for _ExtInt

erichkeane added a comment. In D78785#2010654 , @rjmccall wrote: > Every target does something similar, they just all do it in different ways > because they're mostly written by different people. > > We should restrict this feature to targets where we've

[PATCH] D78785: Fix x86/x86_64 calling convention for _ExtInt

rjmccall added a comment. Every target does something similar, they just all do it in different ways because they're mostly written by different people. We should restrict this feature to targets where we've adequately audited the ABI. It's not a feature until the ABI work is done.

[PATCH] D78785: Fix x86/x86_64 calling convention for _ExtInt

This revision was automatically updated to reflect the committed changes. Closed by commit rG5a1d9c0f5ac8: Fix x86/x86_64 calling convention for _ExtInt (authored by erichkeane). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D78785: Fix x86/x86_64 calling convention for _ExtInt

erichkeane added a comment. In D78785#2010546 , @rjmccall wrote: > Could you answer my question up-thread about whether ExtInt is currently > target-limited? If it isn't, we need to more broadly audit targets. Yikes! I missed that question, I'm sorry

[PATCH] D78785: Fix x86/x86_64 calling convention for _ExtInt

rjmccall added a comment. Could you answer my question up-thread about whether ExtInt is currently target-limited? If it isn't, we need to more broadly audit targets. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78785/new/ https://reviews.llvm.org/D78785

[PATCH] D78785: Fix x86/x86_64 calling convention for _ExtInt

craig.topper accepted this revision. craig.topper added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78785/new/ https://reviews.llvm.org/D78785 ___ cfe-commits mailing list

[PATCH] D78785: Fix x86/x86_64 calling convention for _ExtInt

erichkeane added a comment. @craig.topper Did this look OK/ Is there anything else you want from this? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78785/new/ https://reviews.llvm.org/D78785 ___ cfe-commits mailing list

[PATCH] D78785: Fix x86/x86_64 calling convention for _ExtInt

erichkeane marked an inline comment as done. erichkeane added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:2980 -return (Ty->isPromotableIntegerType() ? ABIArgInfo::getExtend(Ty) - : ABIArgInfo::getDirect()); +

[PATCH] D78785: Fix x86/x86_64 calling convention for _ExtInt

rjmccall added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:2980 -return (Ty->isPromotableIntegerType() ? ABIArgInfo::getExtend(Ty) - : ABIArgInfo::getDirect()); +if (!Ty->isExtIntType()) + return

[PATCH] D78785: Fix x86/x86_64 calling convention for _ExtInt

erichkeane marked an inline comment as done. erichkeane added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:2980 -return (Ty->isPromotableIntegerType() ? ABIArgInfo::getExtend(Ty) - : ABIArgInfo::getDirect()); +

[PATCH] D78785: Fix x86/x86_64 calling convention for _ExtInt

rjmccall added a comment. Is there something currently restricting these types to just x86, or do you need to do a much broader audit? Comment at: clang/lib/CodeGen/TargetInfo.cpp:2980 -return (Ty->isPromotableIntegerType() ? ABIArgInfo::getExtend(Ty) -

[PATCH] D78785: Fix x86/x86_64 calling convention for _ExtInt

erichkeane updated this revision to Diff 259875. erichkeane added a comment. Handle indirect return as well. @craig.topper : Anything else you can think of? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78785/new/ https://reviews.llvm.org/D78785 Files:

[PATCH] D78785: Fix x86/x86_64 calling convention for _ExtInt

erichkeane marked an inline comment as done. erichkeane added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:2804 +return; + } + Quuxplusone wrote: > Asking silly questions because I don't stand a chance of understanding either > the code or

[PATCH] D78785: Fix x86/x86_64 calling convention for _ExtInt

craig.topper added a comment. I think you're missing a check for ExtInt in X86_64ABIInfo::getIndirectReturnResult Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78785/new/ https://reviews.llvm.org/D78785 ___

[PATCH] D78785: Fix x86/x86_64 calling convention for _ExtInt

Quuxplusone added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:2804 +return; + } + Asking silly questions because I don't stand a chance of understanding either the code or the tests... In the x86-64 ABI, an ordinary 16-byte `struct`

[PATCH] D78785: Fix x86/x86_64 calling convention for _ExtInt

erichkeane created this revision. erichkeane added a reviewer: craig.topper. After speaking with Craig Topper about some recent defects, he pointed out that _ExtInts should be passed indirectly if larger than the largest int register, and like ints when smaller than that. This patch implements