Message ID | mptsew55llq.fsf@arm.com |
---|---|
State | New |
Headers | show |
Series | recog: Disallow subregs in mode-punned value [PR115881] | expand |
On 7/19/24 11:37 AM, Richard Sandiford wrote: > In g:9d20529d94b23275885f380d155fe8671ab5353a, I'd extended > insn_propagation to handle simple cases of hard-reg mode punning. > The punned "to" value was created using simplify_subreg rather > than simplify_gen_subreg, on the basis that hard-coded subregs > aren't generally useful after RA (where hard-reg propagation is > expected to happen). > > This PR is about a case where the subreg gets pushed into the > operands of a plus, but the subreg on one of the operands > cannot be simplified. Specifically, we have to generate > (subreg:SI (reg:DI sp) 0) rather than (reg:SI sp), since all > references to the stack pointer must be via stack_pointer_rtx. > > However, code in x86 (reasonably) expects no subregs of registers > to appear after RA, except for special cases like strict_low_part. > This leads to an awkward situation where we can't ban subregs of sp > (because of the strict_low_part use), can't allow direct references > to sp in other modes (because of the stack_pointer_rtx requirement), > and can't allow rvalue uses of the subreg (because of the "no subregs > after RA" assumption). It all seems a bit of a mess... > > I sat on this for a while in the hope that a clean solution might > become apparent, but in the end, I think we'll just have to check > manually for nested subregs and punt on them. > > Tested on aarch64-linux-gnu & x86_64-linux-gnu. OK to install? > > Richard > > > gcc/ > PR rtl-optimization/115881 > * recog.cc: Include rtl-iter.h. > (insn_propagation::apply_to_rvalue_1): Check that the result > of simplify_subreg does not include nested subregs. > > gcc/tetsuite/ > PR rtl-optimization/115881 > * cc.c-torture/compile/pr115881.c: New test. OK jeff
diff --git a/gcc/recog.cc b/gcc/recog.cc index 54b317126c2..23e4820180f 100644 --- a/gcc/recog.cc +++ b/gcc/recog.cc @@ -41,6 +41,7 @@ along with GCC; see the file COPYING3. If not see #include "reload.h" #include "tree-pass.h" #include "function-abi.h" +#include "rtl-iter.h" #ifndef STACK_POP_CODE #if STACK_GROWS_DOWNWARD @@ -1082,6 +1083,7 @@ insn_propagation::apply_to_rvalue_1 (rtx *loc) || !REG_CAN_CHANGE_MODE_P (REGNO (x), GET_MODE (from), GET_MODE (x))) return false; + /* If the reference is paradoxical and the replacement value contains registers, we would need to check that the simplification below does not increase REG_NREGS for those @@ -1090,11 +1092,30 @@ insn_propagation::apply_to_rvalue_1 (rtx *loc) if (paradoxical_subreg_p (GET_MODE (x), GET_MODE (from)) && !CONSTANT_P (to)) return false; + newval = simplify_subreg (GET_MODE (x), to, GET_MODE (from), subreg_lowpart_offset (GET_MODE (x), GET_MODE (from))); if (!newval) return false; + + /* Check that the simplification didn't just push an explicit + subreg down into subexpressions. In particular, for a register + R that has a fixed mode, such as the stack pointer, a subreg of: + + (plus:M (reg:M R) (const_int C)) + + would be: + + (plus:N (subreg:N (reg:M R) ...) (const_int C')) + + But targets can legitimately assume that subregs of hard registers + will not be created after RA (except in special circumstances, + such as strict_low_part). */ + subrtx_iterator::array_type array; + FOR_EACH_SUBRTX (iter, array, newval, NONCONST) + if (GET_CODE (*iter) == SUBREG) + return false; } if (should_unshare) diff --git a/gcc/testsuite/gcc.c-torture/compile/pr115881.c b/gcc/testsuite/gcc.c-torture/compile/pr115881.c new file mode 100644 index 00000000000..8379704c4c8 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/compile/pr115881.c @@ -0,0 +1,16 @@ +typedef unsigned u32; +int list_is_head(); +void tu102_acr_wpr_build_acr_0_0_0(int, long, u32); +void tu102_acr_wpr_build() { + u32 offset = 0; + for (; list_is_head();) { + int hdr; + u32 _addr = offset, _size = sizeof(hdr), *_data = &hdr; + while (_size--) { + tu102_acr_wpr_build_acr_0_0_0(0, _addr, *_data++); + _addr += 4; + } + offset += sizeof(hdr); + } + tu102_acr_wpr_build_acr_0_0_0(0, offset, 0); +}