On Mon, 8 Nov 2021, Jeff Law wrote:
> > Well, the context of this code (around and including hunk #1) is:
> >
> > else if (insn_extra_address_constraint
> > (lookup_constraint (constraints[i])))
> > {
> > address_operand_reloaded[i]
> > = find_reloads_address (recog_data.operand_mode[i], (rtx*) 0,
> > recog_data.operand[i],
> > recog_data.operand_loc[i],
> > i, operand_type[i], ind_levels, insn);
> >
> > /* If we now have a simple operand where we used to have a
> > PLUS or MULT, re-recognize and try again. */
> > if ((OBJECT_P (*recog_data.operand_loc[i])
> > || GET_CODE (*recog_data.operand_loc[i]) == SUBREG)
> > && (GET_CODE (recog_data.operand[i]) == MULT
> > || GET_CODE (recog_data.operand[i]) == PLUS))
> > {
> > INSN_CODE (insn) = -1;
> > retval = find_reloads (insn, replace, ind_levels, live_known,
> > reload_reg_p);
> > return retval;
> > }
> >
> > so the body of the conditional is specifically executed for an address and
> > not a MEM; in this particular case matched with the plain "p" constraint.
> >
> > MEMs are handled with the next conditional right below.
> Ah! Thanks for the clarification. We're digging deep into history here. I
> always thought this code was re-recognizing inside a MEM, but as you note,
> it's
> actually handling stuff outside the MEM, such as a 'p' constraint, which is
> an
> address, but being outside a MEMS means its not subject to the
> mult-by-power-of-2 canonicalization.
>
> So I think the first hunk is fine. There's two others that twiddle
> find_reloads_address_1, which I think can only be reached from
> find_reloads_address. The comment at the front would indicate it's only
> called where AD is inside a MEM.
It's actually hunk #2 that fixes this specific ICE. The other two are
just a consequence: #3 just being a commutative variant of the same case
and #1 from observing that the rtx may now have changed if an ASHIFT too.
> Are we getting into find_reloads_address_1 in any case where the RTL is not an
> address inside a MEM?
I've had a GDB session left open with the problematic source, so it was
merely a case of a rerun and grabbing some data. So with a breakpoint set
at reload.c:5565, conditionalised on (code0 == ASHIFT || code1 == ASHIFT),
we get exactly this, as with my change description:
Breakpoint 52, find_reloads_address_1 (mode=E_DImode, as=0 '\000',
x=0x7fffedbaf7b0, context=0, outer_code=MEM, index_code=SCRATCH,
loc=0x7ffff61a82f0, opnum=1, type=RELOAD_FOR_INPUT, ind_levels=1,
insn=0x7fffefc1c9c0) at .../gcc/reload.c:5565
5565 if (code0 == MULT || code0 == SIGN_EXTEND || code0 == TRUNCATE
(gdb) print code0
$12958 = ASHIFT
(gdb) print code1
$12959 = PLUS
(gdb) print outer_code
$12960 = MEM
(gdb) pr insn
(insn 2051 2050 2052 180 (set (reg/f:SI 0 %r0 [555])
(plus:SI (ashift:SI (reg/v:SI 154 [ n_ctrs ])
(const_int 3 [0x3]))
(plus:SI (reg/v/f:SI 9 %r9 [orig:176 fn_buffer ] [176])
(const_int 24 [0x18])))) ".../libgcc/libgcov-driver.c":172:40
614 {movaddrdi}
(nil))
(gdb) pr x
(plus:SI (ashift:SI (reg/v:SI 154 [ n_ctrs ])
(const_int 3 [0x3]))
(plus:SI (reg/v/f:SI 9 %r9 [orig:176 fn_buffer ] [176])
(const_int 24 [0x18])))
(gdb) bt
#0 find_reloads_address_1 (mode=E_DImode, as=0 '\000', x=0x7fffedbaf7b0,
context=0, outer_code=MEM, index_code=SCRATCH, loc=0x7ffff61a82f0, opnum=1,
type=RELOAD_FOR_INPUT, ind_levels=1, insn=0x7fffefc1c9c0) at
.../gcc/reload.c:5565
#1 0x00000000111ecd18 in find_reloads_address (mode=E_DImode, memrefloc=0x0,
ad=0x7fffedbaf7b0, loc=0x7ffff61a82f0, opnum=1, type=RELOAD_FOR_INPUT,
ind_levels=1, insn=0x7fffefc1c9c0) at .../gcc/reload.c:5264
#2 0x00000000111e2fbc in find_reloads (insn=0x7fffefc1c9c0, replace=1,
ind_levels=1, live_known=1, reload_reg_p=0x12ec7770 <spill_reg_order>) at
.../gcc/reload.c:2843
#3 0x00000000112060f4 in reload_as_needed (live_known=1) at
.../gcc/reload1.c:4522
#4 0x00000000111f9008 in reload (first=0x7ffff5dd3c28, global=1) at
.../gcc/reload1.c:1047
#5 0x0000000010f1458c in do_reload () at .../gcc/ira.c:5944
#6 0x0000000010f14d54 in (anonymous namespace)::pass_reload::execute
(this=0x12f21d20) at .../gcc/ira.c:6118
#7 0x000000001112472c in execute_one_pass (pass=0x12f21d20) at
.../gcc/passes.c:2567
#8 0x0000000011124bc4 in execute_pass_list_1 (pass=0x12f21d20) at
.../gcc/passes.c:2656
#9 0x0000000011124c0c in execute_pass_list_1 (pass=0x12f20b80) at
.../gcc/passes.c:2657
#10 0x0000000011124cac in execute_pass_list (fn=0x7ffff5dc4b00,
pass=0x12f1c900) at .../gcc/passes.c:2667
#11 0x00000000109b64f4 in cgraph_node::expand (this=0x7ffff5d65a50) at
.../gcc/cgraphunit.c:1828
#12 0x00000000109b6eac in expand_all_functions () at .../gcc/cgraphunit.c:1992
#13 0x00000000109b7eb8 in symbol_table::compile (this=0x7ffff5c40000) at
.../gcc/cgraphunit.c:2356
#14 0x00000000109b8638 in symbol_table::finalize_compilation_unit
(this=0x7ffff5c40000) at .../gcc/cgraphunit.c:2537
#15 0x00000000112c4418 in compile_file () at .../gcc/toplev.c:477
#16 0x00000000112c8f60 in do_compile (no_backend=false) at .../gcc/toplev.c:2154
#17 0x00000000112c95d4 in toplev::main (this=0x7fffffffe944, argc=76,
argv=0x7fffffffed78) at .../gcc/toplev.c:2306
#18 0x000000001245a7b8 in main (argc=76, argv=0x7fffffffed78) at
.../gcc/main.c:39
(gdb)
-- so `find_reloads_address' is called from reload.c:2843, which is the
call site in code quoted at the top, for an address associated with the
`p' constraint, and then it goes down to `find_reloads_address_1', which
cannot recognise the rtx and therefore leaves it unchanged.
Here OUTER_CODE is indeed MEM, but it's merely hardcoded by the caller
at reload.c:5264 irrespective of actual insn/rtx:
return find_reloads_address_1 (mode, as, ad, 0, MEM, SCRATCH, loc,
opnum, type, ind_levels, insn);
(I note that `find_reloads_address' does that in several places throughout
and I haven't investigated how legitimate it is, but my guts feeling is at
least in the case concerned it's merely a placeholder, because for a plain
address reference it would have to be nil really.)
Let me know if it clears your concerns and whether there's anything else
you want me to retrieve from that GDB session.
Maciej
