* [PATCH][committed]middle-end intialize regnum in store_bit_field_1 @ 2022-08-30 6:34 Tamar Christina 2022-08-30 6:46 ` Richard Biener 0 siblings, 1 reply; 4+ messages in thread From: Tamar Christina @ 2022-08-30 6:34 UTC (permalink / raw) To: gcc-patches; +Cc: nd, rguenther [-- Attachment #1: Type: text/plain, Size: 986 bytes --] Hi All, This initializes regnum to 0 for when undefined_p. 0 is the right default as it's supposed to get the lowpart when undefined. Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. Ok for master? Thanks, Tamar gcc/ChangeLog: * expmed.cc (store_bit_field_1): Initialize regnum to 0. --- inline copy of patch -- diff --git a/gcc/expmed.cc b/gcc/expmed.cc index 8d7418be418406e72a895ecddf2dc7fdb950c76c..cdc0adb389202a5cab79a8d89056ddc347fb28cb 100644 --- a/gcc/expmed.cc +++ b/gcc/expmed.cc @@ -794,7 +794,7 @@ store_bit_field_1 (rtx str_rtx, poly_uint64 bitsize, poly_uint64 bitnum, words or to cope with mode punning between equal-sized modes. In the latter case, use subreg on the rhs side, not lhs. */ rtx sub; - HOST_WIDE_INT regnum; + HOST_WIDE_INT regnum = 0; poly_uint64 regsize = REGMODE_NATURAL_SIZE (GET_MODE (op0)); if (known_eq (bitnum, 0U) && known_eq (bitsize, GET_MODE_BITSIZE (GET_MODE (op0)))) -- [-- Attachment #2: rb16178.patch --] [-- Type: text/plain, Size: 645 bytes --] diff --git a/gcc/expmed.cc b/gcc/expmed.cc index 8d7418be418406e72a895ecddf2dc7fdb950c76c..cdc0adb389202a5cab79a8d89056ddc347fb28cb 100644 --- a/gcc/expmed.cc +++ b/gcc/expmed.cc @@ -794,7 +794,7 @@ store_bit_field_1 (rtx str_rtx, poly_uint64 bitsize, poly_uint64 bitnum, words or to cope with mode punning between equal-sized modes. In the latter case, use subreg on the rhs side, not lhs. */ rtx sub; - HOST_WIDE_INT regnum; + HOST_WIDE_INT regnum = 0; poly_uint64 regsize = REGMODE_NATURAL_SIZE (GET_MODE (op0)); if (known_eq (bitnum, 0U) && known_eq (bitsize, GET_MODE_BITSIZE (GET_MODE (op0)))) ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][committed]middle-end intialize regnum in store_bit_field_1 2022-08-30 6:34 [PATCH][committed]middle-end intialize regnum in store_bit_field_1 Tamar Christina @ 2022-08-30 6:46 ` Richard Biener 2022-08-30 10:18 ` Richard Sandiford 0 siblings, 1 reply; 4+ messages in thread From: Richard Biener @ 2022-08-30 6:46 UTC (permalink / raw) To: Tamar Christina; +Cc: gcc-patches, nd On Tue, 30 Aug 2022, Tamar Christina wrote: > Hi All, > > This initializes regnum to 0 for when undefined_p. > 0 is the right default as it's supposed to get the lowpart > when undefined. > > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. > > Ok for master? OK. > Thanks, > Tamar > > gcc/ChangeLog: > > * expmed.cc (store_bit_field_1): Initialize regnum to 0. > > --- inline copy of patch -- > diff --git a/gcc/expmed.cc b/gcc/expmed.cc > index 8d7418be418406e72a895ecddf2dc7fdb950c76c..cdc0adb389202a5cab79a8d89056ddc347fb28cb 100644 > --- a/gcc/expmed.cc > +++ b/gcc/expmed.cc > @@ -794,7 +794,7 @@ store_bit_field_1 (rtx str_rtx, poly_uint64 bitsize, poly_uint64 bitnum, > words or to cope with mode punning between equal-sized modes. > In the latter case, use subreg on the rhs side, not lhs. */ > rtx sub; > - HOST_WIDE_INT regnum; > + HOST_WIDE_INT regnum = 0; > poly_uint64 regsize = REGMODE_NATURAL_SIZE (GET_MODE (op0)); > if (known_eq (bitnum, 0U) > && known_eq (bitsize, GET_MODE_BITSIZE (GET_MODE (op0)))) ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][committed]middle-end intialize regnum in store_bit_field_1 2022-08-30 6:46 ` Richard Biener @ 2022-08-30 10:18 ` Richard Sandiford 2022-08-30 10:41 ` Richard Biener 0 siblings, 1 reply; 4+ messages in thread From: Richard Sandiford @ 2022-08-30 10:18 UTC (permalink / raw) To: Richard Biener via Gcc-patches; +Cc: Tamar Christina, Richard Biener, nd Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > On Tue, 30 Aug 2022, Tamar Christina wrote: > >> Hi All, >> >> This initializes regnum to 0 for when undefined_p. >> 0 is the right default as it's supposed to get the lowpart >> when undefined. >> >> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. >> >> Ok for master? > > OK. I'm not sure this is the right fix. We were still supposed to use the correct byte offset in the undefined case. Is the attached OK? Tested on aarch64-linux-gnu. Thanks, Richard ---------------------- store_bit_field_1 tries to convert a field assignment into a subreg assignment. Normally it must check that the field occupies a full word (or more specifically, a full REGMODE_NATURAL_SIZE chunk), so that writing to the subreg doesn't clobber any other fields. But it can skip that check if the structure is known to be in an undefined state. The idea was that, in the undefined case, we could rely on simplify_gen_subreg to do the check for a valid subreg, rather than having to repeat the required endianness logic in the caller. Before the addition of the undefined case, the code could use regnum * regsize to get the byte offset, where regnum came from checking that the start was word-aligned. In the undefined case we need to calculate the byte offset explicitly. gcc/ * expmed.cc (store_bit_field_1): Fix byte offset calculation for undefined structures. --- gcc/expmed.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/gcc/expmed.cc b/gcc/expmed.cc index 8d7418be418..6c02c3bb850 100644 --- a/gcc/expmed.cc +++ b/gcc/expmed.cc @@ -794,7 +794,7 @@ store_bit_field_1 (rtx str_rtx, poly_uint64 bitsize, poly_uint64 bitnum, words or to cope with mode punning between equal-sized modes. In the latter case, use subreg on the rhs side, not lhs. */ rtx sub; - HOST_WIDE_INT regnum; + poly_uint64 bytenum; poly_uint64 regsize = REGMODE_NATURAL_SIZE (GET_MODE (op0)); if (known_eq (bitnum, 0U) && known_eq (bitsize, GET_MODE_BITSIZE (GET_MODE (op0)))) @@ -808,13 +808,13 @@ store_bit_field_1 (rtx str_rtx, poly_uint64 bitsize, poly_uint64 bitnum, return true; } } - else if (((constant_multiple_p (bitnum, regsize * BITS_PER_UNIT, ®num) - && multiple_p (bitsize, regsize * BITS_PER_UNIT)) - || undefined_p) + else if (multiple_p (bitnum, BITS_PER_UNIT, &bytenum) + && (undefined_p + || (multiple_p (bitnum, regsize * BITS_PER_UNIT) + && multiple_p (bitsize, regsize * BITS_PER_UNIT))) && known_ge (GET_MODE_BITSIZE (GET_MODE (op0)), bitsize)) { - sub = simplify_gen_subreg (fieldmode, op0, GET_MODE (op0), - regnum * regsize); + sub = simplify_gen_subreg (fieldmode, op0, GET_MODE (op0), bytenum); if (sub) { if (reverse) -- 2.25.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][committed]middle-end intialize regnum in store_bit_field_1 2022-08-30 10:18 ` Richard Sandiford @ 2022-08-30 10:41 ` Richard Biener 0 siblings, 0 replies; 4+ messages in thread From: Richard Biener @ 2022-08-30 10:41 UTC (permalink / raw) To: Richard Sandiford; +Cc: Richard Biener via Gcc-patches, Tamar Christina, nd On Tue, 30 Aug 2022, Richard Sandiford wrote: > Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > > On Tue, 30 Aug 2022, Tamar Christina wrote: > > > >> Hi All, > >> > >> This initializes regnum to 0 for when undefined_p. > >> 0 is the right default as it's supposed to get the lowpart > >> when undefined. > >> > >> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. > >> > >> Ok for master? > > > > OK. > > I'm not sure this is the right fix. We were still supposed to use the > correct byte offset in the undefined case. > > Is the attached OK? Tested on aarch64-linux-gnu. OK. Thanks, Richard. > Thanks, > Richard > > ---------------------- > > store_bit_field_1 tries to convert a field assignment into a subreg > assignment. Normally it must check that the field occupies a full > word (or more specifically, a full REGMODE_NATURAL_SIZE chunk), > so that writing to the subreg doesn't clobber any other fields. > But it can skip that check if the structure is known to be in > an undefined state. > > The idea was that, in the undefined case, we could rely on > simplify_gen_subreg to do the check for a valid subreg, rather > than having to repeat the required endianness logic in the caller. > > Before the addition of the undefined case, the code could use > regnum * regsize to get the byte offset, where regnum came from > checking that the start was word-aligned. In the undefined case > we need to calculate the byte offset explicitly. > > gcc/ > * expmed.cc (store_bit_field_1): Fix byte offset calculation > for undefined structures. > --- > gcc/expmed.cc | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/gcc/expmed.cc b/gcc/expmed.cc > index 8d7418be418..6c02c3bb850 100644 > --- a/gcc/expmed.cc > +++ b/gcc/expmed.cc > @@ -794,7 +794,7 @@ store_bit_field_1 (rtx str_rtx, poly_uint64 bitsize, poly_uint64 bitnum, > words or to cope with mode punning between equal-sized modes. > In the latter case, use subreg on the rhs side, not lhs. */ > rtx sub; > - HOST_WIDE_INT regnum; > + poly_uint64 bytenum; > poly_uint64 regsize = REGMODE_NATURAL_SIZE (GET_MODE (op0)); > if (known_eq (bitnum, 0U) > && known_eq (bitsize, GET_MODE_BITSIZE (GET_MODE (op0)))) > @@ -808,13 +808,13 @@ store_bit_field_1 (rtx str_rtx, poly_uint64 bitsize, poly_uint64 bitnum, > return true; > } > } > - else if (((constant_multiple_p (bitnum, regsize * BITS_PER_UNIT, ®num) > - && multiple_p (bitsize, regsize * BITS_PER_UNIT)) > - || undefined_p) > + else if (multiple_p (bitnum, BITS_PER_UNIT, &bytenum) > + && (undefined_p > + || (multiple_p (bitnum, regsize * BITS_PER_UNIT) > + && multiple_p (bitsize, regsize * BITS_PER_UNIT))) > && known_ge (GET_MODE_BITSIZE (GET_MODE (op0)), bitsize)) > { > - sub = simplify_gen_subreg (fieldmode, op0, GET_MODE (op0), > - regnum * regsize); > + sub = simplify_gen_subreg (fieldmode, op0, GET_MODE (op0), bytenum); > if (sub) > { > if (reverse) > -- Richard Biener <rguenther@suse.de> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; HRB 36809 (AG Nuernberg) ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-08-30 10:41 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-08-30 6:34 [PATCH][committed]middle-end intialize regnum in store_bit_field_1 Tamar Christina 2022-08-30 6:46 ` Richard Biener 2022-08-30 10:18 ` Richard Sandiford 2022-08-30 10:41 ` Richard Biener
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).