[Bug target/114284] [14 Regression] arm: Load of volatile short gets miscompiled (loaded twice) since r14-8319
jakub at gcc dot gnu.org
gcc-bugzilla@gcc.gnu.org
Fri Mar 8 15:03:37 GMT 2024
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114284
--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
+propagating insn 6 into insn 8, replacing:
+(parallel [
+ (set (reg:SI 114 [ <retval> ])
+ (sign_extend:SI (subreg:HI (reg:SI 117 [ x ]) 0)))
+ (clobber (scratch:SI))
+ ])
+successfully matched this instruction to thumb1_extendhisi2:
+(parallel [
+ (set (reg:SI 114 [ <retval> ])
+ (sign_extend:SI (mem/v/c:HI (reg/f:SI 115) [1 x+0 S2 A16])))
+ (clobber (scratch:SI))
+ ])
I believe before that change we'd never propagate MEM/v, classify_result even
correctly explains why:
4) Creating new (mem/v)s is not correct, since DCE will not remove the old
ones.
But now that fwprop just ignores that or takes it as a slight hint, we get the
invalid change.
I'd actually say all the 4 reasons why it shouldn't be propagating MEMs should
result in don't actually propagate it rather than just a mere hint:
/* Allow (subreg (mem)) -> (mem) simplifications with the following
exceptions:
1) Propagating (mem)s into multiple uses is not profitable.
2) Propagating (mem)s across EBBs may not be profitable if the source EBB
runs less frequently.
3) Propagating (mem)s into paradoxical (subreg)s is not profitable.
4) Creating new (mem/v)s is not correct, since DCE will not remove the old
ones. */
and punt maybe also on propagating any other MEMs into insns.
Though, when check_mem is called, it is called only on the MEMs which are being
propagated, so not sure how to actually check it in there.
Doing
--- a/gcc/fwprop.cc
+++ b/gcc/fwprop.cc
@@ -211,6 +211,11 @@ fwprop_propagation::fwprop_propagation (insn_info
*use_insn,
bool
fwprop_propagation::check_mem (int old_num_changes, rtx mem)
{
+ if (MEM_VOLATILE_P (mem))
+ {
+ failure_reason = "would propagate volatile MEM";
+ return false;
+ }
if (!memory_address_addr_space_p (GET_MODE (mem), XEXP (mem, 0),
MEM_ADDR_SPACE (mem)))
{
doesn't really help, so perhaps I misunderstand what is check_mem used for.
More information about the Gcc-bugs
mailing list