* [PATCH] RISC-V: fix TARGET_PROMOTE_FUNCTION_MODE hook for libcalls
@ 2023-10-31 18:35 Vineet Gupta
2023-10-31 23:18 ` Jeff Law
2023-11-01 19:11 ` Jeff Law
0 siblings, 2 replies; 9+ messages in thread
From: Vineet Gupta @ 2023-10-31 18:35 UTC (permalink / raw)
To: gcc-patches; +Cc: gnu-toolchain, Jeff Law, Vineet Gupta
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.
Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
---
gcc/config/riscv/riscv.cc | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 3e27897d6d30..7b8e9af0a5af 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -8630,9 +8630,10 @@ riscv_promote_function_mode (const_tree type ATTRIBUTE_UNUSED,
return promote_mode (type, mode, punsignedp);
unsignedp = *punsignedp;
- PROMOTE_MODE (as_a <scalar_mode> (mode), unsignedp, type);
+ scalar_mode smode = as_a <scalar_mode> (mode);
+ PROMOTE_MODE (smode, unsignedp, type);
*punsignedp = unsignedp;
- return mode;
+ return smode;
}
/* Implement TARGET_MACHINE_DEPENDENT_REORG. */
--
2.34.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] RISC-V: fix TARGET_PROMOTE_FUNCTION_MODE hook for libcalls
2023-10-31 18:35 [PATCH] RISC-V: fix TARGET_PROMOTE_FUNCTION_MODE hook for libcalls Vineet Gupta
@ 2023-10-31 23:18 ` Jeff Law
2023-10-31 23:41 ` Palmer Dabbelt
2023-11-01 19:11 ` Jeff Law
1 sibling, 1 reply; 9+ messages in thread
From: Jeff Law @ 2023-10-31 23:18 UTC (permalink / raw)
To: Vineet Gupta, gcc-patches; +Cc: gnu-toolchain
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] RISC-V: fix TARGET_PROMOTE_FUNCTION_MODE hook for libcalls
2023-10-31 23:18 ` Jeff Law
@ 2023-10-31 23:41 ` Palmer Dabbelt
2023-11-01 0:51 ` Jeff Law
0 siblings, 1 reply; 9+ messages in thread
From: Palmer Dabbelt @ 2023-10-31 23:41 UTC (permalink / raw)
To: jeffreyalaw; +Cc: Vineet Gupta, gcc-patches, gnu-toolchain
On Tue, 31 Oct 2023 16:18:35 PDT (-0700), jeffreyalaw@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).
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] RISC-V: fix TARGET_PROMOTE_FUNCTION_MODE hook for libcalls
2023-10-31 23:41 ` Palmer Dabbelt
@ 2023-11-01 0:51 ` Jeff Law
2023-11-01 1:37 ` Vineet Gupta
0 siblings, 1 reply; 9+ messages in thread
From: Jeff Law @ 2023-11-01 0:51 UTC (permalink / raw)
To: Palmer Dabbelt; +Cc: Vineet Gupta, gcc-patches, gnu-toolchain
On 10/31/23 17:41, Palmer Dabbelt wrote:
> On Tue, 31 Oct 2023 16:18:35 PDT (-0700), jeffreyalaw@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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] RISC-V: fix TARGET_PROMOTE_FUNCTION_MODE hook for libcalls
2023-11-01 0:51 ` Jeff Law
@ 2023-11-01 1:37 ` Vineet Gupta
0 siblings, 0 replies; 9+ messages in thread
From: Vineet Gupta @ 2023-11-01 1:37 UTC (permalink / raw)
To: Jeff Law, Palmer Dabbelt; +Cc: gcc-patches, gnu-toolchain
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] RISC-V: fix TARGET_PROMOTE_FUNCTION_MODE hook for libcalls
2023-10-31 18:35 [PATCH] RISC-V: fix TARGET_PROMOTE_FUNCTION_MODE hook for libcalls Vineet Gupta
2023-10-31 23:18 ` Jeff Law
@ 2023-11-01 19:11 ` Jeff Law
2023-11-01 19:19 ` Vineet Gupta
2023-11-01 21:51 ` [[Committed]] " Vineet Gupta
1 sibling, 2 replies; 9+ messages in thread
From: Jeff Law @ 2023-11-01 19:11 UTC (permalink / raw)
To: Vineet Gupta, gcc-patches; +Cc: gnu-toolchain
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] RISC-V: fix TARGET_PROMOTE_FUNCTION_MODE hook for libcalls
2023-11-01 19:11 ` Jeff Law
@ 2023-11-01 19:19 ` Vineet Gupta
2023-11-01 21:24 ` Patrick O'Neill
2023-11-01 21:51 ` [[Committed]] " Vineet Gupta
1 sibling, 1 reply; 9+ messages in thread
From: Vineet Gupta @ 2023-11-01 19:19 UTC (permalink / raw)
To: Jeff Law, gcc-patches; +Cc: gnu-toolchain
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] RISC-V: fix TARGET_PROMOTE_FUNCTION_MODE hook for libcalls
2023-11-01 19:19 ` Vineet Gupta
@ 2023-11-01 21:24 ` Patrick O'Neill
0 siblings, 0 replies; 9+ messages in thread
From: Patrick O'Neill @ 2023-11-01 21:24 UTC (permalink / raw)
To: Vineet Gupta, Jeff Law, gcc-patches; +Cc: gnu-toolchain
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 <patrick@rivosinc.com>
Thanks,
Patrick
^ permalink raw reply [flat|nested] 9+ messages in thread
* [[Committed]] RISC-V: fix TARGET_PROMOTE_FUNCTION_MODE hook for libcalls
2023-11-01 19:11 ` Jeff Law
2023-11-01 19:19 ` Vineet Gupta
@ 2023-11-01 21:51 ` Vineet Gupta
1 sibling, 0 replies; 9+ messages in thread
From: Vineet Gupta @ 2023-11-01 21:51 UTC (permalink / raw)
To: gcc-patches; +Cc: gnu-toolchain, Vineet Gupta, Patrick O'Neill
Fixes: 3496ca4e6566 ("RISC-V: Add runtime invariant support")
riscv_promote_function_mode doesn't promote a SI to DI for libcalls
case. It intends to do that however the code is broken (regression).
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.
gcc/ChangeLog:
* config/riscv/riscv.cc (riscv_promote_function_mode): Fix mode
returned for libcall case.
Tested-by: Patrick O'Neill <patrick@rivosinc.com> # pre-commit-CI #526
Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
---
gcc/config/riscv/riscv.cc | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 0148a4f2e431..895a098cd9a6 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -8630,9 +8630,10 @@ riscv_promote_function_mode (const_tree type ATTRIBUTE_UNUSED,
return promote_mode (type, mode, punsignedp);
unsignedp = *punsignedp;
- PROMOTE_MODE (as_a <scalar_mode> (mode), unsignedp, type);
+ scalar_mode smode = as_a <scalar_mode> (mode);
+ PROMOTE_MODE (smode, unsignedp, type);
*punsignedp = unsignedp;
- return mode;
+ return smode;
}
/* Implement TARGET_MACHINE_DEPENDENT_REORG. */
--
2.34.1
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-11-01 21:51 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-31 18:35 [PATCH] RISC-V: fix TARGET_PROMOTE_FUNCTION_MODE hook for libcalls Vineet Gupta
2023-10-31 23:18 ` Jeff Law
2023-10-31 23:41 ` Palmer Dabbelt
2023-11-01 0:51 ` Jeff Law
2023-11-01 1:37 ` Vineet Gupta
2023-11-01 19:11 ` Jeff Law
2023-11-01 19:19 ` Vineet Gupta
2023-11-01 21:24 ` Patrick O'Neill
2023-11-01 21:51 ` [[Committed]] " Vineet Gupta
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).