Vineet Gupta <vine...@rivosinc.com> writes:
> On 6/3/25 10:11, Richard Sandiford wrote:
>> Vineet Gupta <vine...@rivosinc.com> writes:
>>> On 6/3/25 08:24, Richard Sandiford wrote:
>>>> I think the issue is that:
>>>>
>>>> (insn 9 8 27 2 (parallel [
>>>> (asm_operands/v ("fsrm %0") ("") 0 [
>>>> (reg:SI 15 a5 [139])
>>>> ]
>>>> [
>>>> (asm_input:SI ("r") frm-run-1.c:33)
>>>> ]
>>>> [] frm-run-1.c:33)
>>>> (clobber (reg:V4096QI 69 frm))
>>>> ]) "frm-run-1.c":33:3 -1
>>>> (nil))
>>>>
>>>> is seen as invalidating FRM and so:
>>>>
>>>> (insn 27 9 28 2 (set (reg:SI 15 a5 [144])
>>>> (reg:SI 69 frm)) "frm-run-1.c":43:1 2829 {frrmsi}
>>>> (nil))
>>>>
>>>> is seen as an uninitialised read. I suppose clobbers in inline asms
>>>> need to be treated as real definitions rather than just kills.
>>> In general or specifically inside of late_combine ?
>> We can start with the routines that rtl-ssa uses for its dataflow analysis.
>> Does the patch below help?
>
> Sorry, it doesn't. It seems the new code is not getting hit because @has_asm
> is
> false.
> I tried a little hack to force it for my use case but it doesn't doesn't hit
> the
> new code.
>
> @@ -2236,7 +2236,7 @@ rtx_properties::try_to_add_src (const_rtx x, unsigned
> int
> flags)
> }
> else if (code == UNSPEC_VOLATILE)
> has_volatile_refs = true;
> - else if (code == ASM_INPUT || code == ASM_OPERANDS)
> + else if (code == ASM_INPUT || code == ASM_OPERANDS || code == CLOBBER)
>
> Next I tried another mindless hack
>
> @@ -2295,7 +2295,7 @@ rtx_properties::try_to_add_pattern (const_rtx pat)
> case CLOBBER:
> {
> rtx x = XEXP (pat, 0);
> - if (has_asm && asm_clobber_is_read_modify_write_p (x))
> + if (asm_clobber_is_read_modify_write_p (x))
>
> Now the new code hits, but the final outcome is still the same, late_combine2
> is
> still eliminating those.
Are you sure? Removing the "has_asm &&" works for me on your
frm-mode-switch/upstream-v2 branch.
But I think the non-hacky fix for the has_asm problem is the one below.
Richard
rtx_properties::try_to_add_pattern had a special case for
ASM_OPERANDS. But this was a mistake, since:
(a) it subverted the handling of ASM_OPERANDS in try_to_add_src.
(b) it meant that there was an unnecessary divergence between the
handling of asms with output operands and those without.
I think this was probably something that I added early on,
based on patterns elsewhere in the file, and didn't revisit later
when the patch evolved.
gcc/
* rtlanal.cc (rtx_properties::try_to_add_pattern): Remove special
handling of ASM_OPERANDS and defer to try_to_add_src instead.
---
gcc/rtlanal.cc | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/gcc/rtlanal.cc b/gcc/rtlanal.cc
index 239d6691c4c..91cefae69be 100644
--- a/gcc/rtlanal.cc
+++ b/gcc/rtlanal.cc
@@ -2256,11 +2256,6 @@ rtx_properties::try_to_add_pattern (const_rtx pat)
break;
}
- case ASM_OPERANDS:
- for (int i = 0, len = ASM_OPERANDS_INPUT_LENGTH (pat); i < len; ++i)
- try_to_add_src (ASM_OPERANDS_INPUT (pat, i));
- break;
-
case CLOBBER:
try_to_add_dest (XEXP (pat, 0), rtx_obj_flags::IS_CLOBBER);
break;
@@ -2274,6 +2269,8 @@ rtx_properties::try_to_add_pattern (const_rtx pat)
/* All the other possibilities never store and can use a normal
rtx walk. This includes:
+ - ASM_INPUT
+ - ASM_OPERANDS
- USE
- TRAP_IF
- PREFETCH
--
2.43.0
