Re: [PATCH] RISC-V: fix TARGET_PROMOTE_FUNCTION_MODE hook for libcalls
On 11/1/23 12:19, Vineet Gupta wrote: On 11/1/23 12:11, Jeff Law wrote: On 10/31/23 12:35, Vineet Gupta wrote: riscv_promote_function_mode doesn't promote a SI to DI for libcalls case. The fix is what generic promote_mode () in explow.cc does. I really don't understand why the old code didn't work, but stepping thru the debugger shows old code didn't and fixed does. This showed up when testing Ajit's REE ABI extension series which probes the ABI (using a NULL tree type) and ends up hitting the libcall code path. [Usual caveat, I'll wait for Pre-commit CI to run the tests and report] gcc/ChangeLog: * config/riscv/riscv.cc (riscv_promote_function_mode): Fix mode returned for libcall case. Macro that change their arguments are evil ;( I think Juzhe's patch in this space a few months back wasn't supposed to change behavior. Oh, its a regression. I can add a Fixes: tag OK once CI finishes without regressions. Thx, -Vineet It passes precommit CI without any new failures: https://github.com/ewlu/gcc-precommit-ci/issues/526#issuecomment-1787891174 Tested-by: Patrick O'Neill Thanks, Patrick
Re: [PATCH] RISC-V: fix TARGET_PROMOTE_FUNCTION_MODE hook for libcalls
On 11/1/23 12:11, Jeff Law wrote: On 10/31/23 12:35, Vineet Gupta wrote: riscv_promote_function_mode doesn't promote a SI to DI for libcalls case. The fix is what generic promote_mode () in explow.cc does. I really don't understand why the old code didn't work, but stepping thru the debugger shows old code didn't and fixed does. This showed up when testing Ajit's REE ABI extension series which probes the ABI (using a NULL tree type) and ends up hitting the libcall code path. [Usual caveat, I'll wait for Pre-commit CI to run the tests and report] gcc/ChangeLog: * config/riscv/riscv.cc (riscv_promote_function_mode): Fix mode returned for libcall case. Macro that change their arguments are evil ;( I think Juzhe's patch in this space a few months back wasn't supposed to change behavior. Oh, its a regression. I can add a Fixes: tag OK once CI finishes without regressions. Thx, -Vineet
Re: [PATCH] RISC-V: fix TARGET_PROMOTE_FUNCTION_MODE hook for libcalls
On 10/31/23 12:35, Vineet Gupta wrote: riscv_promote_function_mode doesn't promote a SI to DI for libcalls case. The fix is what generic promote_mode () in explow.cc does. I really don't understand why the old code didn't work, but stepping thru the debugger shows old code didn't and fixed does. This showed up when testing Ajit's REE ABI extension series which probes the ABI (using a NULL tree type) and ends up hitting the libcall code path. [Usual caveat, I'll wait for Pre-commit CI to run the tests and report] gcc/ChangeLog: * config/riscv/riscv.cc (riscv_promote_function_mode): Fix mode returned for libcall case. Macro that change their arguments are evil ;( I think Juzhe's patch in this space a few months back wasn't supposed to change behavior. OK once CI finishes without regressions. jeff
Re: [PATCH] RISC-V: fix TARGET_PROMOTE_FUNCTION_MODE hook for libcalls
On 10/31/23 17:51, Jeff Law wrote: We also have a non-orthogonality in the ABI sign extension rules between SI and DI, a few of us were talking about it on the internal slack (though the specifics were for a different patch, Vineet has a few in flight). So the old issue I was thinking of really only affects targets that push arguments on the stack and when a sub-word push actually allocates a full word on the stack (m68k, but !coldfire, h8 and probably others of that era). Point being, those issues don't apply here. OK, I think Palmer was conflating this with the discussion in other thread/patch. -Vineet
Re: [PATCH] RISC-V: fix TARGET_PROMOTE_FUNCTION_MODE hook for libcalls
On 10/31/23 17:41, Palmer Dabbelt wrote: On Tue, 31 Oct 2023 16:18:35 PDT (-0700), jeffreya...@gmail.com wrote: On 10/31/23 12:35, Vineet Gupta wrote: riscv_promote_function_mode doesn't promote a SI to DI for libcalls case. The fix is what generic promote_mode () in explow.cc does. I really don't understand why the old code didn't work, but stepping thru the debugger shows old code didn't and fixed does. This showed up when testing Ajit's REE ABI extension series which probes the ABI (using a NULL tree type) and ends up hitting the libcall code path. [Usual caveat, I'll wait for Pre-commit CI to run the tests and report] gcc/ChangeLog: * config/riscv/riscv.cc (riscv_promote_function_mode): Fix mode returned for libcall case. Hmm. There may be dragons in here. I'll need to find and review an old conversation in this space (libcalls and argument promotions). We also have a non-orthogonality in the ABI sign extension rules between SI and DI, a few of us were talking about it on the internal slack (though the specifics were for a different patch, Vineet has a few in flight). So the old issue I was thinking of really only affects targets that push arguments on the stack and when a sub-word push actually allocates a full word on the stack (m68k, but !coldfire, h8 and probably others of that era). Point being, those issues don't apply here. jeff
Re: [PATCH] RISC-V: fix TARGET_PROMOTE_FUNCTION_MODE hook for libcalls
On Tue, 31 Oct 2023 16:18:35 PDT (-0700), jeffreya...@gmail.com wrote: On 10/31/23 12:35, Vineet Gupta wrote: riscv_promote_function_mode doesn't promote a SI to DI for libcalls case. The fix is what generic promote_mode () in explow.cc does. I really don't understand why the old code didn't work, but stepping thru the debugger shows old code didn't and fixed does. This showed up when testing Ajit's REE ABI extension series which probes the ABI (using a NULL tree type) and ends up hitting the libcall code path. [Usual caveat, I'll wait for Pre-commit CI to run the tests and report] gcc/ChangeLog: * config/riscv/riscv.cc (riscv_promote_function_mode): Fix mode returned for libcall case. Hmm. There may be dragons in here. I'll need to find and review an old conversation in this space (libcalls and argument promotions). We also have a non-orthogonality in the ABI sign extension rules between SI and DI, a few of us were talking about it on the internal slack (though the specifics were for a different patch, Vineet has a few in flight).
Re: [PATCH] RISC-V: fix TARGET_PROMOTE_FUNCTION_MODE hook for libcalls
On 10/31/23 12:35, Vineet Gupta wrote: riscv_promote_function_mode doesn't promote a SI to DI for libcalls case. The fix is what generic promote_mode () in explow.cc does. I really don't understand why the old code didn't work, but stepping thru the debugger shows old code didn't and fixed does. This showed up when testing Ajit's REE ABI extension series which probes the ABI (using a NULL tree type) and ends up hitting the libcall code path. [Usual caveat, I'll wait for Pre-commit CI to run the tests and report] gcc/ChangeLog: * config/riscv/riscv.cc (riscv_promote_function_mode): Fix mode returned for libcall case. Hmm. There may be dragons in here. I'll need to find and review an old conversation in this space (libcalls and argument promotions). Jeff
