[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-08-04 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In D124435#3701163 , @jyknight wrote: > In D124435#3697259 , @rjmccall > wrote: > >> I know what you're saying, but I don't think it matches any model of how >> programmers use command

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-08-04 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. In D124435#3697259 , @rjmccall wrote: > I know what you're saying, but I don't think it matches any model of how > programmers use command line flags. You're imagining that a programmer sits > down and considers all of their f

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-08-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. I know what you're saying, but I don't think it matches any model of how programmers use command line flags. You're imagining that a programmer sits down and considers all of their flags deeply and holistically before touching any of them, and that's just not how thes

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-08-03 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. In D124435#3696862 , @rjmccall wrote: > That is an interesting idea. I like that it integrates this into > `-fclang-abi-compat`. The way that `-mno-conservative-small-integer-abi` > ends up meaning opposite things based on th

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-08-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. That is an interesting idea. I like that it integrates this into `-fclang-abi-compat`. The way that `-mno-conservative-small-integer-abi` ends up meaning opposite things based on the abi-compat setting worries me a lot, though. Repository: rG LLVM Github Monorepo

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-08-02 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. Thinking more about the naming issue, I think the real issue is that the proposed options are an implementation detail, not what we should expose to users. I realize that the non-boolean option was proposed by rjmmccall before, but I'd suggest that we go back to someth

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-08-02 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. It looks like you haven't implemented the target-specific logic for this yet. I cannot let you commit until you do that, because you will be breaking the ABI on Apple platforms. Comment at: clang/include/clang/Basic/CodeGenOptions.h:150 +Assumed

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-08-02 Thread LiuChen via Phabricator via cfe-commits
LiuChen3 added a comment. In D124435#3515541 , @jyknight wrote: > I find the option names you have a bit confusing. I'd like to suggest calling > them, instead: > > caller: Extend a small integer parameter in the caller; callee will assume it > has alre

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-08-02 Thread LiuChen via Phabricator via cfe-commits
LiuChen3 added a comment. Sorry for the late update. I hope you can take the time to continue reviewing, @rjmccall . I'm not sure if I understand you correctly. Now if the `-mextend-small-integers=default` is used, compiler will report error. Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-08-02 Thread LiuChen via Phabricator via cfe-commits
LiuChen3 updated this revision to Diff 449230. LiuChen3 added a comment. rebase and address rjmccall's comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124435/new/ https://reviews.llvm.org/D124435 Files: clang/docs/ClangCommandLineReferenc

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-05-16 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. I'm fine with alternatives on the names. I just don't think we want names that imply that the caller and callee are parallel here, as if there were some sort of requirement that the callee always extends. In those conventions, the callee gets passed 8 or 16 meaningfu

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-05-16 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. I find the option names you have a bit confusing. I'd like to suggest calling them, instead: caller: Extend a small integer parameter in the caller; callee will assume it has already been extended. callee : Pass a small integer parameter directly in caller, extend in c

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-05-16 Thread LiuChen via Phabricator via cfe-commits
LiuChen3 added a comment. Thanks, @rjmccall . I'm sorry I don't have much time on this patch recently. I will update it later. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124435/new/ https://reviews.llvm.org/D124435

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-05-09 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: clang/docs/ClangCommandLineReference.rst:3028 +extend the small integer argument in both caller and callee. Using other +value can disable this, which may improve performance and code size. + > Specifies how to handle s

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-05-06 Thread LiuChen via Phabricator via cfe-commits
LiuChen3 updated this revision to Diff 427551. LiuChen3 added a comment. Use `-mextend-small-integers=` instead of boolean option Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124435/new/ https://reviews.llvm.org/D124435 Files: clang/docs/ClangC

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-04-27 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:1938 +return IsConservativeExtend ? ABIArgInfo::getConservativeExtend(Ty) +: ABIArgInfo::getExtend(Ty); } LiuChen3 wrote: > rjmccall wrote: > > Liu

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-04-27 Thread LiuChen via Phabricator via cfe-commits
LiuChen3 added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:1938 +return IsConservativeExtend ? ABIArgInfo::getConservativeExtend(Ty) +: ABIArgInfo::getExtend(Ty); } rjmccall wrote: > LiuChen3 wrote: > > rjm

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-04-27 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: clang/docs/ClangCommandLineReference.rst:2988-2992 +.. option:: -mconservative-extend +Always extend the integer parameter both in the callee and caller. + +.. option:: -mno-conservative-extend +Keep the original integer parameter passi

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-04-27 Thread LiuChen via Phabricator via cfe-commits
LiuChen3 added inline comments. Comment at: clang/docs/ClangCommandLineReference.rst:2988-2992 +.. option:: -mconservative-extend +Always extend the integer parameter both in the callee and caller. + +.. option:: -mno-conservative-extend +Keep the original integer parameter passi

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-04-27 Thread LiuChen via Phabricator via cfe-commits
LiuChen3 added a comment. In D124435#3474130 , @skan wrote: > Should we update the `clang/docs/ReleaseNotes.rst` for this? The ReleaseNotes says "written by LLVM Team". So I am not sure if I can update this. Repository: rG LLVM Github Monorepo CHAN

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-04-27 Thread LiuChen via Phabricator via cfe-commits
LiuChen3 updated this revision to Diff 425428. LiuChen3 added a comment. add one missing comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124435/new/ https://reviews.llvm.org/D124435 Files: clang/docs/ClangCommandLineReference.rst clang/i

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-04-27 Thread LiuChen via Phabricator via cfe-commits
LiuChen3 marked 9 inline comments as done. LiuChen3 added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2310 case ABIArgInfo::Extend: + case ABIArgInfo::ConservativeExtend: if (RetAI.isSignExt()) At present, `ConservativeExtend` has no specif

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-04-27 Thread LiuChen via Phabricator via cfe-commits
LiuChen3 updated this revision to Diff 425427. LiuChen3 added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124435/new/ https://reviews.llvm.org/D124435 Files: clang/docs/ClangCommandLineReference.rst clang/include/

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-04-26 Thread LiuChen via Phabricator via cfe-commits
LiuChen3 added inline comments. Comment at: clang/docs/ClangCommandLineReference.rst:2988-2992 +.. option:: -mconservative-extend +Always extend the integer parameter both in the callee and caller. + +.. option:: -mno-conservative-extend +Keep the original integer parameter passi

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-04-26 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: clang/docs/ClangCommandLineReference.rst:2988-2992 +.. option:: -mconservative-extend +Always extend the integer parameter both in the callee and caller. + +.. option:: -mno-conservative-extend +Keep the original integer parameter passi

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-04-26 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: clang/include/clang/CodeGen/CGFunctionInfo.h:120 bool SRetAfterThis : 1; // isIndirect() bool InReg : 1; // isDirect() || isExtend() || isIndirect() bool CanBeFlattened: 1; // isDirect() Instead of int

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-04-26 Thread LiuChen via Phabricator via cfe-commits
LiuChen3 added inline comments. Comment at: clang/test/CodeGen/X86/integer_argument_passing.c:2 +// RUN: %clang_cc1 -O2 -triple -x86_64-linux-gnu %s -emit-llvm -o - | FileCheck %s --check-prefixes=EXTEND,CHECK +// RUN: %clang_cc1 -O2 -triple -i386-linux-gnu %s -emit-llvm -o - |

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-04-26 Thread Phoebe Wang via Phabricator via cfe-commits
pengfei added inline comments. Comment at: clang/test/CodeGen/X86/integer_argument_passing.c:2 +// RUN: %clang_cc1 -O2 -triple -x86_64-linux-gnu %s -emit-llvm -o - | FileCheck %s --check-prefixes=EXTEND,CHECK +// RUN: %clang_cc1 -O2 -triple -i386-linux-gnu %s -emit-llvm -o - | F

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-04-26 Thread Phoebe Wang via Phabricator via cfe-commits
pengfei added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2451 + // attribute to the callee. + if (AttrOnCallSite || AI.getKind() == ABIArgInfo::Extend) { +if (AI.isSignExt()) LiuChen3 wrote: > pengfei wrote: > > Does the change af

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-04-26 Thread LiuChen via Phabricator via cfe-commits
LiuChen3 added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2451 + // attribute to the callee. + if (AttrOnCallSite || AI.getKind() == ABIArgInfo::Extend) { +if (AI.isSignExt()) pengfei wrote: > Does the change affect Windows? Seems

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-04-26 Thread Phoebe Wang via Phabricator via cfe-commits
pengfei added inline comments. Comment at: clang/docs/ClangCommandLineReference.rst:2988-2992 +.. option:: -mconservative-extend +Always extend the integer parameter both in the callee and caller. + +.. option:: -mno-conservative-extend +Keep the original integer parameter passin

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-04-26 Thread LiuChen via Phabricator via cfe-commits
LiuChen3 added a comment. In D124435#3474130 , @skan wrote: > Should we update the `clang/docs/ReleaseNotes.rst` for this? Maybe? I will update it in the next patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-04-26 Thread Kan Shengchen via Phabricator via cfe-commits
skan added a comment. Should we update the `clang/docs/ReleaseNotes.rst` for this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124435/new/ https://reviews.llvm.org/D124435 ___ cfe-commits mailing list

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-04-26 Thread LiuChen via Phabricator via cfe-commits
LiuChen3 updated this revision to Diff 425135. LiuChen3 added a comment. fix bug Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124435/new/ https://reviews.llvm.org/D124435 Files: clang/docs/ClangCommandLineReference.rst clang/include/clang/Bas

[PATCH] D124435: [X86] Always extend the integer parameters in callee

2022-04-26 Thread LiuChen via Phabricator via cfe-commits
LiuChen3 created this revision. Herald added subscribers: dexonsmith, jdoerfert, pengfei. Herald added a project: All. LiuChen3 requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. For now clang will assume the integer parameters have be