From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27062 invoked by alias); 18 Oct 2012 21:18:31 -0000 Received: (qmail 27039 invoked by uid 22791); 18 Oct 2012 21:18:30 -0000 X-SWARE-Spam-Status: No, hits=-4.3 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,KHOP_RCVD_TRUST,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE X-Spam-Check-By: sourceware.org Received: from mail-we0-f175.google.com (HELO mail-we0-f175.google.com) (74.125.82.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 18 Oct 2012 21:18:17 +0000 Received: by mail-we0-f175.google.com with SMTP id t44so5785636wey.20 for ; Thu, 18 Oct 2012 14:18:16 -0700 (PDT) Received: by 10.180.85.99 with SMTP id g3mr13986499wiz.5.1350595095915; Thu, 18 Oct 2012 14:18:15 -0700 (PDT) Received: from localhost ([2.26.188.227]) by mx.google.com with ESMTPS id a10sm35696514wiz.4.2012.10.18.14.18.06 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 18 Oct 2012 14:18:10 -0700 (PDT) From: Richard Sandiford To: Eric Botcazou Mail-Followup-To: Eric Botcazou ,gcc-patches@gcc.gnu.org, rdsandiford@googlemail.com Cc: gcc-patches@gcc.gnu.org Subject: Re: Tidy store_bit_field_1 & co. References: <87obk4lu63.fsf@talisman.home> <8248446.zrYi3fceYI@polaris> Date: Thu, 18 Oct 2012 21:35:00 -0000 In-Reply-To: <8248446.zrYi3fceYI@polaris> (Eric Botcazou's message of "Wed, 17 Oct 2012 22:27:58 +0200") Message-ID: <87d30fjx7y.fsf@talisman.home> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2012-10/txt/msg01758.txt.bz2 Hi Eric, Thanks for the review. Eric Botcazou writes: >> -476,34 +468,36 @@ store_bit_field_1 (rtx str_rtx, unsigne >> } >> >> /* If the target is a register, overwriting the entire object, or storing >> - a full-word or multi-word field can be done with just a SUBREG. + >> a full-word or multi-word field can be done with just a SUBREG. */ + if >> (!MEM_P (op0) >> + && bitsize == GET_MODE_BITSIZE (fieldmode) >> + && ((bitsize % BITS_PER_WORD == 0 >> + && bitnum % BITS_PER_WORD == 0) >> + || (bitsize == GET_MODE_BITSIZE (GET_MODE (op0)) >> + && bitnum == 0))) >> + { >> + /* Use the subreg machinery either to narrow OP0 to the required >> + words or to cope with mode punning between equal-sized modes. */ >> + rtx sub = simplify_gen_subreg (fieldmode, op0, GET_MODE (op0), >> + bitnum / BITS_PER_UNIT); >> + if (sub) >> + { >> + emit_move_insn (sub, value); >> + return true; >> + } >> + } > > I'd use the following variant for the sake of symmetry with the MEM_P case: > > if (!MEM_P (op0) > && bitnum % BITS_PER_WORD == 0 > && bitsize == GET_MODE_BITSIZE (fieldmode) > && [...] OK. >> - /* If OP0 is a register, BITPOS must count within a word. >> - But as we have it, it counts within whatever size OP0 now has. >> - On a bigendian machine, these are not the same, so convert. */ >> - if (BYTES_BIG_ENDIAN >> - && !MEM_P (op0) >> - && unit > GET_MODE_BITSIZE (GET_MODE (op0))) >> - bitpos += unit - GET_MODE_BITSIZE (GET_MODE (op0)); >> - >> /* Storing an lsb-aligned field in a register >> - can be done with a movestrict instruction. */ >> + can be done with a movstrict instruction. */ >> >> if (!MEM_P (op0) >> - && (BYTES_BIG_ENDIAN ? bitpos + bitsize == unit : bitpos == 0) >> + && (BYTES_BIG_ENDIAN >> + ? bitnum + bitsize == GET_MODE_BITSIZE (GET_MODE (op0)) >> + : bitnum == 0) >> && bitsize == GET_MODE_BITSIZE (fieldmode) >> && optab_handler (movstrict_optab, fieldmode) != CODE_FOR_nothing) >> { >> @@ -558,8 +546,7 @@ store_bit_field_1 (rtx str_rtx, unsigned >> arg0 = SUBREG_REG (arg0); >> } >> >> - subreg_off = (bitnum % BITS_PER_WORD) / BITS_PER_UNIT >> - + (offset * UNITS_PER_WORD); >> + subreg_off = bitnum / BITS_PER_UNIT; >> if (validate_subreg (fieldmode, GET_MODE (arg0), arg0, subreg_off)) >> { >> arg0 = gen_rtx_SUBREG (fieldmode, arg0, subreg_off); > > Aren't you losing movstrict opportunities in big-endian mode? If GET_MODE > (op0) is 2 words and bitnum + bitsize == BITS_PER_WORD. Argh, good catch. I hadn't realised that it could accept lowparts of individual words. The revised patch below adds a lowpart_bit_field_p function with the condition that originally appeared in the extract_bit_field_1 patch. >> @@ -638,34 +625,6 @@ store_bit_field_1 (rtx str_rtx, unsigned >> return true; >> } >> >> - /* From here on we can assume that the field to be stored in is >> - a full-word (whatever type that is), since it is shorter than a word. >> */ - >> - /* OFFSET is the number of words or bytes (UNIT says which) >> - from STR_RTX to the first word or byte containing part of the field. >> */ - >> - if (!MEM_P (op0)) >> - { >> - if (offset != 0 >> - || GET_MODE_SIZE (GET_MODE (op0)) > UNITS_PER_WORD) >> - { >> - if (!REG_P (op0)) >> - { >> - /* Since this is a destination (lvalue), we can't copy >> - it to a pseudo. We can remove a SUBREG that does not >> - change the size of the operand. Such a SUBREG may >> - have been added above. */ >> - gcc_assert (GET_CODE (op0) == SUBREG >> - && (GET_MODE_SIZE (GET_MODE (op0)) >> - == GET_MODE_SIZE (GET_MODE (SUBREG_REG (op0))))); >> - op0 = SUBREG_REG (op0); >> - } >> - op0 = gen_rtx_SUBREG (mode_for_size (BITS_PER_WORD, MODE_INT, 0), >> - op0, (offset * UNITS_PER_WORD)); >> - } >> - offset = 0; >> - } >> - >> /* If VALUE has a floating-point or complex mode, access it as an >> integer of the corresponding size. This can occur on a machine >> with 64 bit registers that uses SFmode for float. It can also >> @@ -679,9 +638,30 @@ store_bit_field_1 (rtx str_rtx, unsigned >> emit_move_insn (gen_lowpart (GET_MODE (orig_value), value), >> orig_value); } >> >> - /* Now OFFSET is nonzero only if OP0 is memory >> - and is therefore always measured in bytes. */ >> + /* If OP0 is a multi-word register, narrow it to the affected word. >> + If the region spans two words, defer to store_split_bit_field. */ >> + if (!MEM_P (op0) && GET_MODE_SIZE (GET_MODE (op0)) > UNITS_PER_WORD) >> + { >> + op0 = simplify_gen_subreg (word_mode, op0, GET_MODE (op0), >> + bitnum / BITS_PER_WORD * UNITS_PER_WORD); >> + gcc_assert (op0); >> + bitnum %= BITS_PER_WORD; >> + if (bitnum + bitsize > BITS_PER_WORD) >> + { >> + if (!fallback_p) >> + return false; >> + >> + store_split_bit_field (op0, bitsize, bitnum, bitregion_start, >> + bitregion_end, value); >> + return true; >> + } >> + } >> + >> + /* From here on we can assume that the field to be stored in fits >> + within a word. If the destination is a register, it too fits >> + in a word. */ > > I presume the reasoning is that the offset != 0 test is redundant with the > test on the mode size because of the REG_P && ... early exit? Yeah. I was also worried that generating an out-of-range subreg seemed more likely to lead to an ICE-on-invalid than something like a negative shift would. >> mode = GET_MODE (op0); >> - >> - /* Now MODE is either some integral mode for a MEM as OP0, >> - or is a full-word for a REG as OP0. TOTAL_BITS corresponds. >> - The bit field is contained entirely within OP0. >> - BITPOS is the starting bit number within OP0. >> - (OP0's mode may actually be narrower than MODE.) */ >> + gcc_assert (SCALAR_INT_MODE_P (mode)); >> + /* Note that bitsize + bitnum can be greater than GET_MODE_BITSIZE (mode) >> + for invalid input, such as f5 from gcc.dg/pr48335-2.c. */ > > Blank line after the assertion. OK. >> if (BYTES_BIG_ENDIAN) >> - /* BITPOS is the distance between our msb >> - and that of the containing datum. >> - Convert it to the distance from the lsb. */ >> - bitpos = total_bits - bitsize - bitpos; >> + /* BITNUM is the distance between our msb >> + and that of the containing datum. >> + Convert it to the distance from the lsb. */ >> + bitnum = GET_MODE_BITSIZE (mode) - bitsize - bitnum; > > So bitnum can be negative? Is that new or pre-existing? It's pre-existing. As you probably guessed, I started out trying to add an assert that bitnum + bitsize was "sane", but hit a failure on that particular testcase. But the same thing happened before my patch. The bitnum is in range (24 bits into a 32-bit value), it's the sum of the two that's bad (40 bits, even though only 32 are available). Tested in the same way as before. Richard gcc/ * expmed.c (lowpart_bit_field_p): New function. (store_bit_field_1): Remove unit, offset, bitpos and byte_offset from the outermost scope. Express conditions in terms of bitnum rather than offset, bitpos and byte_offset. Split the plain move cases into two, one for memory accesses and one for register accesses. Allow simplify_gen_subreg to fail rather than calling validate_subreg. Move the handling of multiword OP0s after the code that coerces VALUE to an integer mode. Use simplify_gen_subreg for this case and assert that it succeeds. If the field still spans several words, pass it directly to store_split_bit_field. Assume after that point that both sources and register targets fit within a word. Replace x-prefixed variables with non-prefixed forms. Compute the bitpos for insv register operands directly in the chosen unit size, rather than going through an intermediate BITS_PER_WORD unit size. Update the call to store_fixed_bit_field. (store_fixed_bit_field): Replace the bitpos and offset parameters with a single bitnum parameter, of the same form as store_bit_field. Assume that OP0 contains the full field. Simplify the memory offset calculation. Assert that the processed OP0 has an integral mode. (store_split_bit_field): Update the call to store_fixed_bit_field. Index: gcc/expmed.c =================================================================== --- gcc/expmed.c 2012-10-15 20:27:52.865652696 +0100 +++ gcc/expmed.c 2012-10-18 19:10:29.268718181 +0100 @@ -49,7 +49,6 @@ static void store_fixed_bit_field (rtx, unsigned HOST_WIDE_INT, unsigned HOST_WIDE_INT, unsigned HOST_WIDE_INT, - unsigned HOST_WIDE_INT, rtx); static void store_split_bit_field (rtx, unsigned HOST_WIDE_INT, unsigned HOST_WIDE_INT, @@ -393,6 +392,23 @@ mode_for_extraction (enum extraction_pat return word_mode; return data->operand[opno].mode; } + +/* Return true if a bitfield of size BITSIZE at bit number BITNUM within + a structure of mode STRUCT_MODE represents a lowpart subreg. The subreg + offset is then BITNUM / BITS_PER_UNIT. */ + +static bool +lowpart_bit_field_p (unsigned HOST_WIDE_INT bitnum, + unsigned HOST_WIDE_INT bitsize, + enum machine_mode struct_mode) +{ + if (BYTES_BIG_ENDIAN) + return (bitnum % BITS_PER_UNIT + && (bitnum + bitsize == GET_MODE_BITSIZE (struct_mode) + || (bitnum + bitsize) % BITS_PER_WORD == 0)); + else + return bitnum % BITS_PER_WORD == 0; +} /* A subroutine of store_bit_field, with the same arguments. Return true if the operation could be implemented. @@ -409,15 +425,9 @@ store_bit_field_1 (rtx str_rtx, unsigned enum machine_mode fieldmode, rtx value, bool fallback_p) { - unsigned int unit - = (MEM_P (str_rtx)) ? BITS_PER_UNIT : BITS_PER_WORD; - unsigned HOST_WIDE_INT offset, bitpos; rtx op0 = str_rtx; - int byte_offset; rtx orig_value; - enum machine_mode op_mode = mode_for_extraction (EP_insv, 3); - while (GET_CODE (op0) == SUBREG) { /* The following line once was done only if WORDS_BIG_ENDIAN, @@ -427,8 +437,7 @@ store_bit_field_1 (rtx str_rtx, unsigned always get higher addresses. */ int inner_mode_size = GET_MODE_SIZE (GET_MODE (SUBREG_REG (op0))); int outer_mode_size = GET_MODE_SIZE (GET_MODE (op0)); - - byte_offset = 0; + int byte_offset = 0; /* Paradoxical subregs need special handling on big endian machines. */ if (SUBREG_BYTE (op0) == 0 && inner_mode_size < outer_mode_size) @@ -476,34 +485,35 @@ store_bit_field_1 (rtx str_rtx, unsigned } /* If the target is a register, overwriting the entire object, or storing - a full-word or multi-word field can be done with just a SUBREG. + a full-word or multi-word field can be done with just a SUBREG. */ + if (!MEM_P (op0) + && bitnum % BITS_PER_WORD == 0 + && bitsize == GET_MODE_BITSIZE (fieldmode) + && (bitsize == GET_MODE_BITSIZE (GET_MODE (op0)) + || bitsize % BITS_PER_WORD == 0)) + { + /* Use the subreg machinery either to narrow OP0 to the required + words or to cope with mode punning between equal-sized modes. */ + rtx sub = simplify_gen_subreg (fieldmode, op0, GET_MODE (op0), + bitnum / BITS_PER_UNIT); + if (sub) + { + emit_move_insn (sub, value); + return true; + } + } - If the target is memory, storing any naturally aligned field can be + /* If the target is memory, storing any naturally aligned field can be done with a simple store. For targets that support fast unaligned memory, any naturally sized, unit aligned field can be done directly. */ - - offset = bitnum / unit; - bitpos = bitnum % unit; - byte_offset = (bitnum % BITS_PER_WORD) / BITS_PER_UNIT - + (offset * UNITS_PER_WORD); - - if (bitpos == 0 + if (MEM_P (op0) + && bitnum % BITS_PER_UNIT == 0 && bitsize == GET_MODE_BITSIZE (fieldmode) - && (!MEM_P (op0) - ? ((GET_MODE_SIZE (fieldmode) >= UNITS_PER_WORD - || GET_MODE_SIZE (GET_MODE (op0)) == GET_MODE_SIZE (fieldmode)) - && ((GET_MODE (op0) == fieldmode && byte_offset == 0) - || validate_subreg (fieldmode, GET_MODE (op0), op0, - byte_offset))) - : (! SLOW_UNALIGNED_ACCESS (fieldmode, MEM_ALIGN (op0)) - || (offset * BITS_PER_UNIT % bitsize == 0 - && MEM_ALIGN (op0) % GET_MODE_BITSIZE (fieldmode) == 0)))) - { - if (MEM_P (op0)) - op0 = adjust_bitfield_address (op0, fieldmode, offset); - else if (GET_MODE (op0) != fieldmode) - op0 = simplify_gen_subreg (fieldmode, op0, GET_MODE (op0), - byte_offset); + && (!SLOW_UNALIGNED_ACCESS (fieldmode, MEM_ALIGN (op0)) + || (bitnum % bitsize == 0 + && MEM_ALIGN (op0) % bitsize == 0))) + { + op0 = adjust_bitfield_address (op0, fieldmode, bitnum / BITS_PER_UNIT); emit_move_insn (op0, value); return true; } @@ -526,19 +536,11 @@ store_bit_field_1 (rtx str_rtx, unsigned } } - /* If OP0 is a register, BITPOS must count within a word. - But as we have it, it counts within whatever size OP0 now has. - On a bigendian machine, these are not the same, so convert. */ - if (BYTES_BIG_ENDIAN - && !MEM_P (op0) - && unit > GET_MODE_BITSIZE (GET_MODE (op0))) - bitpos += unit - GET_MODE_BITSIZE (GET_MODE (op0)); - /* Storing an lsb-aligned field in a register - can be done with a movestrict instruction. */ + can be done with a movstrict instruction. */ if (!MEM_P (op0) - && (BYTES_BIG_ENDIAN ? bitpos + bitsize == unit : bitpos == 0) + && lowpart_bit_field_p (bitnum, bitsize, GET_MODE (op0)) && bitsize == GET_MODE_BITSIZE (fieldmode) && optab_handler (movstrict_optab, fieldmode) != CODE_FOR_nothing) { @@ -558,8 +560,7 @@ store_bit_field_1 (rtx str_rtx, unsigned arg0 = SUBREG_REG (arg0); } - subreg_off = (bitnum % BITS_PER_WORD) / BITS_PER_UNIT - + (offset * UNITS_PER_WORD); + subreg_off = bitnum / BITS_PER_UNIT; if (validate_subreg (fieldmode, GET_MODE (arg0), arg0, subreg_off)) { arg0 = gen_rtx_SUBREG (fieldmode, arg0, subreg_off); @@ -638,34 +639,6 @@ store_bit_field_1 (rtx str_rtx, unsigned return true; } - /* From here on we can assume that the field to be stored in is - a full-word (whatever type that is), since it is shorter than a word. */ - - /* OFFSET is the number of words or bytes (UNIT says which) - from STR_RTX to the first word or byte containing part of the field. */ - - if (!MEM_P (op0)) - { - if (offset != 0 - || GET_MODE_SIZE (GET_MODE (op0)) > UNITS_PER_WORD) - { - if (!REG_P (op0)) - { - /* Since this is a destination (lvalue), we can't copy - it to a pseudo. We can remove a SUBREG that does not - change the size of the operand. Such a SUBREG may - have been added above. */ - gcc_assert (GET_CODE (op0) == SUBREG - && (GET_MODE_SIZE (GET_MODE (op0)) - == GET_MODE_SIZE (GET_MODE (SUBREG_REG (op0))))); - op0 = SUBREG_REG (op0); - } - op0 = gen_rtx_SUBREG (mode_for_size (BITS_PER_WORD, MODE_INT, 0), - op0, (offset * UNITS_PER_WORD)); - } - offset = 0; - } - /* If VALUE has a floating-point or complex mode, access it as an integer of the corresponding size. This can occur on a machine with 64 bit registers that uses SFmode for float. It can also @@ -679,9 +652,30 @@ store_bit_field_1 (rtx str_rtx, unsigned emit_move_insn (gen_lowpart (GET_MODE (orig_value), value), orig_value); } - /* Now OFFSET is nonzero only if OP0 is memory - and is therefore always measured in bytes. */ + /* If OP0 is a multi-word register, narrow it to the affected word. + If the region spans two words, defer to store_split_bit_field. */ + if (!MEM_P (op0) && GET_MODE_SIZE (GET_MODE (op0)) > UNITS_PER_WORD) + { + op0 = simplify_gen_subreg (word_mode, op0, GET_MODE (op0), + bitnum / BITS_PER_WORD * UNITS_PER_WORD); + gcc_assert (op0); + bitnum %= BITS_PER_WORD; + if (bitnum + bitsize > BITS_PER_WORD) + { + if (!fallback_p) + return false; + + store_split_bit_field (op0, bitsize, bitnum, bitregion_start, + bitregion_end, value); + return true; + } + } + + /* From here on we can assume that the field to be stored in fits + within a word. If the destination is a register, it too fits + in a word. */ + enum machine_mode op_mode = mode_for_extraction (EP_insv, 3); if (HAVE_insv && GET_MODE (value) != BLKmode && bitsize > 0 @@ -690,25 +684,34 @@ store_bit_field_1 (rtx str_rtx, unsigned -fstrict-volatile-bitfields is in effect. */ && !(MEM_P (op0) && MEM_VOLATILE_P (op0) && flag_strict_volatile_bitfields > 0) - && ! ((REG_P (op0) || GET_CODE (op0) == SUBREG) - && (bitsize + bitpos > GET_MODE_BITSIZE (op_mode))) /* Do not use insv if the bit region is restricted and op_mode integer at offset doesn't fit into the restricted region. */ && !(MEM_P (op0) && bitregion_end - && bitnum - bitpos + GET_MODE_BITSIZE (op_mode) + && bitnum - (bitnum % BITS_PER_UNIT) + GET_MODE_BITSIZE (op_mode) > bitregion_end + 1)) { struct expand_operand ops[4]; - int xbitpos = bitpos; + unsigned HOST_WIDE_INT bitpos = bitnum; rtx value1; rtx xop0 = op0; rtx last = get_last_insn (); bool copy_back = false; - /* Add OFFSET into OP0's address. */ + unsigned int unit = GET_MODE_BITSIZE (op_mode); if (MEM_P (xop0)) - xop0 = adjust_bitfield_address (xop0, byte_mode, offset); + { + /* Get a reference to the first byte of the field. */ + xop0 = adjust_bitfield_address (xop0, byte_mode, + bitpos / BITS_PER_UNIT); + bitpos %= BITS_PER_UNIT; + } + else + { + /* Convert from counting within OP0 to counting in OP_MODE. */ + if (BYTES_BIG_ENDIAN) + bitpos += unit - GET_MODE_BITSIZE (GET_MODE (op0)); + } /* If xop0 is a register, we need it in OP_MODE to make it acceptable to the format of insv. */ @@ -735,20 +738,13 @@ store_bit_field_1 (rtx str_rtx, unsigned copy_back = true; } - /* We have been counting XBITPOS within UNIT. - Count instead within the size of the register. */ - if (BYTES_BIG_ENDIAN && !MEM_P (xop0)) - xbitpos += GET_MODE_BITSIZE (op_mode) - unit; - - unit = GET_MODE_BITSIZE (op_mode); - /* If BITS_BIG_ENDIAN is zero on a BYTES_BIG_ENDIAN machine, we count "backwards" from the size of the unit we are inserting into. Otherwise, we count bits from the most significant on a BYTES/BITS_BIG_ENDIAN machine. */ if (BITS_BIG_ENDIAN != BYTES_BIG_ENDIAN) - xbitpos = unit - bitsize - xbitpos; + bitpos = unit - bitsize - bitpos; /* Convert VALUE to op_mode (which insv insn wants) in VALUE1. */ value1 = value; @@ -787,7 +783,7 @@ store_bit_field_1 (rtx str_rtx, unsigned create_fixed_operand (&ops[0], xop0); create_integer_operand (&ops[1], bitsize); - create_integer_operand (&ops[2], xbitpos); + create_integer_operand (&ops[2], bitpos); create_input_operand (&ops[3], value1, op_mode); if (maybe_expand_insn (CODE_FOR_insv, 4, ops)) { @@ -832,7 +828,8 @@ store_bit_field_1 (rtx str_rtx, unsigned && GET_MODE_BITSIZE (bestmode) > MEM_ALIGN (op0))) { rtx last, tempreg, xop0; - unsigned HOST_WIDE_INT xoffset, xbitpos; + unsigned int unit; + unsigned HOST_WIDE_INT offset, bitpos; last = get_last_insn (); @@ -840,14 +837,14 @@ store_bit_field_1 (rtx str_rtx, unsigned that mode. Compute the offset as a multiple of this unit, counting in bytes. */ unit = GET_MODE_BITSIZE (bestmode); - xoffset = (bitnum / unit) * GET_MODE_SIZE (bestmode); - xbitpos = bitnum % unit; - xop0 = adjust_bitfield_address (op0, bestmode, xoffset); + offset = (bitnum / unit) * GET_MODE_SIZE (bestmode); + bitpos = bitnum % unit; + xop0 = adjust_bitfield_address (op0, bestmode, offset); /* Fetch that unit, store the bitfield in it, then store the unit. */ tempreg = copy_to_reg (xop0); - if (store_bit_field_1 (tempreg, bitsize, xbitpos, + if (store_bit_field_1 (tempreg, bitsize, bitpos, bitregion_start, bitregion_end, fieldmode, orig_value, false)) { @@ -861,8 +858,8 @@ store_bit_field_1 (rtx str_rtx, unsigned if (!fallback_p) return false; - store_fixed_bit_field (op0, offset, bitsize, bitpos, - bitregion_start, bitregion_end, value); + store_fixed_bit_field (op0, bitsize, bitnum, bitregion_start, + bitregion_end, value); return true; } @@ -918,25 +915,17 @@ store_bit_field (rtx str_rtx, unsigned H gcc_unreachable (); } -/* Use shifts and boolean operations to store VALUE - into a bit field of width BITSIZE - in a memory location specified by OP0 except offset by OFFSET bytes. - (OFFSET must be 0 if OP0 is a register.) - The field starts at position BITPOS within the byte. - (If OP0 is a register, it may be a full word or a narrower mode, - but BITPOS still counts within a full word, - which is significant on bigendian machines.) */ +/* Use shifts and boolean operations to store VALUE into a bit field of + width BITSIZE in OP0, starting at bit BITNUM. */ static void -store_fixed_bit_field (rtx op0, unsigned HOST_WIDE_INT offset, - unsigned HOST_WIDE_INT bitsize, - unsigned HOST_WIDE_INT bitpos, +store_fixed_bit_field (rtx op0, unsigned HOST_WIDE_INT bitsize, + unsigned HOST_WIDE_INT bitnum, unsigned HOST_WIDE_INT bitregion_start, unsigned HOST_WIDE_INT bitregion_end, rtx value) { enum machine_mode mode; - unsigned int total_bits = BITS_PER_WORD; rtx temp; int all_zero = 0; int all_one = 0; @@ -948,19 +937,7 @@ store_fixed_bit_field (rtx op0, unsigned and a field split across two bytes. Such cases are not supposed to be able to occur. */ - if (REG_P (op0) || GET_CODE (op0) == SUBREG) - { - gcc_assert (!offset); - /* Special treatment for a bit field split across two registers. */ - if (bitsize + bitpos > BITS_PER_WORD) - { - store_split_bit_field (op0, bitsize, bitpos, - bitregion_start, bitregion_end, - value); - return; - } - } - else + if (MEM_P (op0)) { unsigned HOST_WIDE_INT maxbits = MAX_FIXED_MODE_SIZE; @@ -983,58 +960,39 @@ store_fixed_bit_field (rtx op0, unsigned && flag_strict_volatile_bitfields > 0) mode = GET_MODE (op0); else - mode = get_best_mode (bitsize, bitpos + offset * BITS_PER_UNIT, - bitregion_start, bitregion_end, + mode = get_best_mode (bitsize, bitnum, bitregion_start, bitregion_end, MEM_ALIGN (op0), mode, MEM_VOLATILE_P (op0)); if (mode == VOIDmode) { /* The only way this should occur is if the field spans word boundaries. */ - store_split_bit_field (op0, bitsize, bitpos + offset * BITS_PER_UNIT, - bitregion_start, bitregion_end, value); + store_split_bit_field (op0, bitsize, bitnum, bitregion_start, + bitregion_end, value); return; } - total_bits = GET_MODE_BITSIZE (mode); - - /* Make sure bitpos is valid for the chosen mode. Adjust BITPOS to - be in the range 0 to total_bits-1, and put any excess bytes in - OFFSET. */ - if (bitpos >= total_bits) - { - offset += (bitpos / total_bits) * (total_bits / BITS_PER_UNIT); - bitpos -= ((bitpos / total_bits) * (total_bits / BITS_PER_UNIT) - * BITS_PER_UNIT); - } - - /* Get ref to an aligned byte, halfword, or word containing the field. - Adjust BITPOS to be position within a word, - and OFFSET to be the offset of that word. - Then alter OP0 to refer to that word. */ - bitpos += (offset % (total_bits / BITS_PER_UNIT)) * BITS_PER_UNIT; - offset -= (offset % (total_bits / BITS_PER_UNIT)); - op0 = adjust_bitfield_address (op0, mode, offset); + HOST_WIDE_INT bit_offset = bitnum - bitnum % GET_MODE_BITSIZE (mode); + op0 = adjust_bitfield_address (op0, mode, bit_offset / BITS_PER_UNIT); + bitnum -= bit_offset; } mode = GET_MODE (op0); + gcc_assert (SCALAR_INT_MODE_P (mode)); - /* Now MODE is either some integral mode for a MEM as OP0, - or is a full-word for a REG as OP0. TOTAL_BITS corresponds. - The bit field is contained entirely within OP0. - BITPOS is the starting bit number within OP0. - (OP0's mode may actually be narrower than MODE.) */ + /* Note that bitsize + bitnum can be greater than GET_MODE_BITSIZE (mode) + for invalid input, such as f5 from gcc.dg/pr48335-2.c. */ if (BYTES_BIG_ENDIAN) - /* BITPOS is the distance between our msb - and that of the containing datum. - Convert it to the distance from the lsb. */ - bitpos = total_bits - bitsize - bitpos; + /* BITNUM is the distance between our msb + and that of the containing datum. + Convert it to the distance from the lsb. */ + bitnum = GET_MODE_BITSIZE (mode) - bitsize - bitnum; - /* Now BITPOS is always the distance between our lsb + /* Now BITNUM is always the distance between our lsb and that of OP0. */ - /* Shift VALUE left by BITPOS bits. If VALUE is not constant, + /* Shift VALUE left by BITNUM bits. If VALUE is not constant, we must first convert its mode to MODE. */ if (CONST_INT_P (value)) @@ -1051,12 +1009,12 @@ store_fixed_bit_field (rtx op0, unsigned || (bitsize == HOST_BITS_PER_WIDE_INT && v == -1)) all_one = 1; - value = lshift_value (mode, value, bitpos, bitsize); + value = lshift_value (mode, value, bitnum, bitsize); } else { int must_and = (GET_MODE_BITSIZE (GET_MODE (value)) != bitsize - && bitpos + bitsize != GET_MODE_BITSIZE (mode)); + && bitnum + bitsize != GET_MODE_BITSIZE (mode)); if (GET_MODE (value) != mode) value = convert_to_mode (mode, value, 1); @@ -1065,9 +1023,9 @@ store_fixed_bit_field (rtx op0, unsigned value = expand_binop (mode, and_optab, value, mask_rtx (mode, 0, bitsize, 0), NULL_RTX, 1, OPTAB_LIB_WIDEN); - if (bitpos > 0) + if (bitnum > 0) value = expand_shift (LSHIFT_EXPR, mode, value, - bitpos, NULL_RTX, 1); + bitnum, NULL_RTX, 1); } /* Now clear the chosen bits in OP0, @@ -1080,7 +1038,7 @@ store_fixed_bit_field (rtx op0, unsigned if (! all_one) { temp = expand_binop (mode, and_optab, temp, - mask_rtx (mode, bitpos, bitsize, 1), + mask_rtx (mode, bitnum, bitsize, 1), NULL_RTX, 1, OPTAB_LIB_WIDEN); temp = force_reg (mode, temp); } @@ -1235,12 +1193,11 @@ store_split_bit_field (rtx op0, unsigned else word = op0; - /* OFFSET is in UNITs, and UNIT is in bits. - store_fixed_bit_field wants offset in bytes. If WORD is const0_rtx, + /* OFFSET is in UNITs, and UNIT is in bits. If WORD is const0_rtx, it is just an out-of-bounds access. Ignore it. */ if (word != const0_rtx) - store_fixed_bit_field (word, offset * unit / BITS_PER_UNIT, thissize, - thispos, bitregion_start, bitregion_end, part); + store_fixed_bit_field (word, thissize, offset * unit + thispos, + bitregion_start, bitregion_end, part); bitsdone += thissize; } }