[RS6000] PR54009 again
My PR54131 fix caused PR54009 to raise its ugly head again. Allowing all LO_SUM addresses in the 'Y' constraint (mem_operand_gpr) wasn't quite correct. We still need to check for wrap when offsetting for multiple words/regs. Fixing that means the LO_SUM addresses not accepted by 'Y' need an address reload. OK assuming powerpc64 and powerpc-linux regression testing passes? PR target/54009 * config/rs6000/rs6000.c (mem_operand_gpr): Check that LO_SUM addresses won't wrap when offsetting. (rs6000_secondary_reload): Provide secondary reloads needed for wrapping LO_SUM addresses. Index: gcc/config/rs6000/rs6000.c === --- gcc/config/rs6000/rs6000.c (revision 195707) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -5135,17 +5135,14 @@ mem_operand_gpr if (TARGET_POWERPC64 (offset 3) != 0) return false; + extra = GET_MODE_SIZE (mode) - UNITS_PER_WORD; + gcc_assert (extra = 0); + if (GET_CODE (addr) == LO_SUM) -/* We know by alignment that ABI_AIX medium/large model toc refs - will not cross a 32k boundary, since all entries in the - constant pool are naturally aligned and we check alignment for - other medium model toc-relative addresses. For ABI_V4 and - ABI_DARWIN lo_sum addresses, we just check that 64-bit - offsets are 4-byte aligned. */ -return true; +/* For lo_sum addresses, we must allow any offset except one that + causes a wrap, so test only the low 16 bits. */ +offset = ((offset 0x) ^ 0x8000) - 0x8000; - extra = GET_MODE_SIZE (mode) - UNITS_PER_WORD; - gcc_assert (extra = 0); return offset + 0x8000 0x1u - extra; } @@ -13823,19 +13819,31 @@ rs6000_secondary_reload MEM_P (x) GET_MODE_SIZE (GET_MODE (x)) = UNITS_PER_WORD) { - rtx off = address_offset (XEXP (x, 0)); - unsigned int extra = GET_MODE_SIZE (GET_MODE (x)) - UNITS_PER_WORD; + rtx addr = XEXP (x, 0); + rtx off = address_offset (addr); - if (off != NULL_RTX - (INTVAL (off) 3) != 0 - (unsigned HOST_WIDE_INT) INTVAL (off) + 0x8000 0x1 - extra) + if (off != NULL_RTX) { - if (in_p) - sri-icode = CODE_FOR_reload_di_load; + unsigned int extra = GET_MODE_SIZE (GET_MODE (x)) - UNITS_PER_WORD; + unsigned HOST_WIDE_INT offset = INTVAL (off); + + /* We need a secondary reload when our legitimate_address_p +says the address is good (as otherwise the entire address +will be reloaded), and the offset is not a multiple of +four. */ + if ((GET_CODE (addr) == LO_SUM + || offset + 0x8000 0x1 - extra) + (offset 3) != 0) + { + if (in_p) + sri-icode = CODE_FOR_reload_di_load; + else + sri-icode = CODE_FOR_reload_di_store; + sri-extra_cost = 2; + ret = NO_REGS; + } else - sri-icode = CODE_FOR_reload_di_store; - sri-extra_cost = 2; - ret = NO_REGS; + default_p = true; } else default_p = true; @@ -13845,25 +13853,43 @@ rs6000_secondary_reload MEM_P (x) GET_MODE_SIZE (GET_MODE (x)) UNITS_PER_WORD) { - rtx off = address_offset (XEXP (x, 0)); - unsigned int extra = GET_MODE_SIZE (GET_MODE (x)) - UNITS_PER_WORD; + rtx addr = XEXP (x, 0); + rtx off = address_offset (addr); - /* We need a secondary reload only when our legitimate_address_p -says the address is good (as otherwise the entire address -will be reloaded). So for mode sizes of 8 and 16 this will -be when the offset is in the ranges [0x7ffc,0x7fff] and -[0x7ff4,0x7ff7] respectively. Note that the address we see -here may have been manipulated by legitimize_reload_address. */ - if (off != NULL_RTX - ((unsigned HOST_WIDE_INT) INTVAL (off) - (0x8000 - extra) - UNITS_PER_WORD)) + if (off != NULL_RTX) { - if (in_p) - sri-icode = CODE_FOR_reload_si_load; + unsigned int extra = GET_MODE_SIZE (GET_MODE (x)) - UNITS_PER_WORD; + unsigned HOST_WIDE_INT offset = INTVAL (off); + + /* We need a secondary reload when our legitimate_address_p +says the address is good (as otherwise the entire address +will be reloaded), and we have a wrap. + +legitimate_lo_sum_address_p allows LO_SUM addresses to +have any offset so test for wrap in the low 16 bits. + +legitimate_offset_address_p checks for the range +[-0x8000,0x7fff] for mode size of 8 and [-0x8000,0x7ff7] +for mode size of 16. We wrap at [0x7ffc,0x7fff] and +[0x7ff4,0x7fff] respectively, so test for the +intersection of these
Re: [RS6000] PR54009 again
On Wed, Feb 6, 2013 at 8:38 AM, Alan Modra amo...@gmail.com wrote: My PR54131 fix caused PR54009 to raise its ugly head again. Allowing all LO_SUM addresses in the 'Y' constraint (mem_operand_gpr) wasn't quite correct. We still need to check for wrap when offsetting for multiple words/regs. Fixing that means the LO_SUM addresses not accepted by 'Y' need an address reload. OK assuming powerpc64 and powerpc-linux regression testing passes? PR target/54009 * config/rs6000/rs6000.c (mem_operand_gpr): Check that LO_SUM addresses won't wrap when offsetting. (rs6000_secondary_reload): Provide secondary reloads needed for wrapping LO_SUM addresses. Okay. We just need to tweak it until we close these corner cases. Do you have a testcase that can be added? Thanks, David
Re: [RS6000] PR54009 again
On Wed, Feb 06, 2013 at 01:24:43PM -0500, David Edelsohn wrote: We just need to tweak it until we close these corner cases. Do you have a testcase that can be added? David's corner case comment prompted me to look at this again, and sure enough, there was another case that could be triggered with -m32 -mpowerpc64. Discussed off-list with David, final patch as follows. Committed revision 195835 (pr54131 test) and 195836. gcc/ PR target/54009 * config/rs6000/rs6000.c (mem_operand_gpr): Check that LO_SUM addresses won't wrap when offsetting. (rs6000_secondary_reload): Provide secondary reloads needed for wrapping LO_SUM addresses. gcc/testsuite/ PR target/54131 * gfortran.dg/pr54131.f: New test. PR target/54009 * gcc.target/powerpc/pr54009.c: New test. Index: gcc/config/rs6000/rs6000.c === --- gcc/config/rs6000/rs6000.c (revision 195707) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -5135,17 +5135,14 @@ if (TARGET_POWERPC64 (offset 3) != 0) return false; + extra = GET_MODE_SIZE (mode) - UNITS_PER_WORD; + gcc_assert (extra = 0); + if (GET_CODE (addr) == LO_SUM) -/* We know by alignment that ABI_AIX medium/large model toc refs - will not cross a 32k boundary, since all entries in the - constant pool are naturally aligned and we check alignment for - other medium model toc-relative addresses. For ABI_V4 and - ABI_DARWIN lo_sum addresses, we just check that 64-bit - offsets are 4-byte aligned. */ -return true; +/* For lo_sum addresses, we must allow any offset except one that + causes a wrap, so test only the low 16 bits. */ +offset = ((offset 0x) ^ 0x8000) - 0x8000; - extra = GET_MODE_SIZE (mode) - UNITS_PER_WORD; - gcc_assert (extra = 0); return offset + 0x8000 0x1u - extra; } @@ -13823,19 +13819,36 @@ MEM_P (x) GET_MODE_SIZE (GET_MODE (x)) = UNITS_PER_WORD) { - rtx off = address_offset (XEXP (x, 0)); - unsigned int extra = GET_MODE_SIZE (GET_MODE (x)) - UNITS_PER_WORD; + rtx addr = XEXP (x, 0); + rtx off = address_offset (addr); - if (off != NULL_RTX - (INTVAL (off) 3) != 0 - (unsigned HOST_WIDE_INT) INTVAL (off) + 0x8000 0x1 - extra) + if (off != NULL_RTX) { - if (in_p) - sri-icode = CODE_FOR_reload_di_load; + unsigned int extra = GET_MODE_SIZE (GET_MODE (x)) - UNITS_PER_WORD; + unsigned HOST_WIDE_INT offset = INTVAL (off); + + /* We need a secondary reload when our legitimate_address_p +says the address is good (as otherwise the entire address +will be reloaded), and the offset is not a multiple of +four or we have an address wrap. Address wrap will only +occur for LO_SUMs since legitimate_offset_address_p +rejects addresses for 16-byte mems that will wrap. */ + if (GET_CODE (addr) == LO_SUM + ? (1 /* legitimate_address_p allows any offset for lo_sum */ + ((offset 3) != 0 +|| ((offset 0x) ^ 0x8000) = 0x1 - extra)) + : (offset + 0x8000 0x1 - extra /* legitimate_address_p */ + (offset 3) != 0)) + { + if (in_p) + sri-icode = CODE_FOR_reload_di_load; + else + sri-icode = CODE_FOR_reload_di_store; + sri-extra_cost = 2; + ret = NO_REGS; + } else - sri-icode = CODE_FOR_reload_di_store; - sri-extra_cost = 2; - ret = NO_REGS; + default_p = true; } else default_p = true; @@ -13845,25 +13858,43 @@ MEM_P (x) GET_MODE_SIZE (GET_MODE (x)) UNITS_PER_WORD) { - rtx off = address_offset (XEXP (x, 0)); - unsigned int extra = GET_MODE_SIZE (GET_MODE (x)) - UNITS_PER_WORD; + rtx addr = XEXP (x, 0); + rtx off = address_offset (addr); - /* We need a secondary reload only when our legitimate_address_p -says the address is good (as otherwise the entire address -will be reloaded). So for mode sizes of 8 and 16 this will -be when the offset is in the ranges [0x7ffc,0x7fff] and -[0x7ff4,0x7ff7] respectively. Note that the address we see -here may have been manipulated by legitimize_reload_address. */ - if (off != NULL_RTX - ((unsigned HOST_WIDE_INT) INTVAL (off) - (0x8000 - extra) - UNITS_PER_WORD)) + if (off != NULL_RTX) { - if (in_p) - sri-icode = CODE_FOR_reload_si_load; + unsigned int extra = GET_MODE_SIZE (GET_MODE (x)) - UNITS_PER_WORD; + unsigned HOST_WIDE_INT offset = INTVAL (off); + + /* We need a secondary reload when our legitimate_address_p +