* [RS6000] pr65810, powerpc64 alignment of r2 @ 2015-04-20 13:53 Alan Modra 2015-04-20 14:27 ` Jakub Jelinek 2015-04-20 17:11 ` David Edelsohn 0 siblings, 2 replies; 6+ messages in thread From: Alan Modra @ 2015-04-20 13:53 UTC (permalink / raw) To: gcc-patches; +Cc: David Edelsohn This fixes a thinko in offsettable_ok_by_alignment. It's not the absolute placement that matters, but the toc-pointer relative offset. So alignment of r2 also needs to be taken into account. Bootstrapped and regression tested powerpc64-linux. OK for mainline and gcc-5 branch? Without the dead code removal for the branch.. I also have a linker fix to align the toc pointer and gcc configury changes to supply POWERPC64_TOC_POINTER_ALIGNMENT in the works. PR target/65810 * config/rs6000/rs6000.c (POWERPC64_TOC_POINTER_ALIGNMENT): Define. (offsettable_ok_by_alignment): Return false if size exceeds POWERPC64_TOC_POINTER_ALIGNMENT. Replace dead code with assertion. Index: gcc/config/rs6000/rs6000.c =================================================================== --- gcc/config/rs6000/rs6000.c (revision 222227) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -6497,13 +6497,21 @@ virtual_stack_registers_memory_p (rtx op) } /* Return true if a MODE sized memory accesses to OP plus OFFSET - is known to not straddle a 32k boundary. */ + is known to not straddle a 32k boundary. This function is used + to determine whether -mcmodel=medium code can use TOC pointer + relative addressing for OP. This means the alignment of the TOC + pointer must also be taken into account, and unfortunately that is + only 8 bytes. */ +#ifndef POWERPC64_TOC_POINTER_ALIGNMENT +#define POWERPC64_TOC_POINTER_ALIGNMENT 8 +#endif + static bool offsettable_ok_by_alignment (rtx op, HOST_WIDE_INT offset, machine_mode mode) { - tree decl, type; + tree decl; unsigned HOST_WIDE_INT dsize, dalign, lsb, mask; if (GET_CODE (op) != SYMBOL_REF) @@ -6510,6 +6518,8 @@ offsettable_ok_by_alignment (rtx op, HOST_WIDE_INT return false; dsize = GET_MODE_SIZE (mode); + if (dsize > POWERPC64_TOC_POINTER_ALIGNMENT) + return false; decl = SYMBOL_REF_DECL (op); if (!decl) { @@ -6553,6 +6563,8 @@ offsettable_ok_by_alignment (rtx op, HOST_WIDE_INT return false; dsize = tree_to_uhwi (DECL_SIZE_UNIT (decl)); + if (dsize > POWERPC64_TOC_POINTER_ALIGNMENT) + return false; if (dsize > 32768) return false; @@ -6560,32 +6572,8 @@ offsettable_ok_by_alignment (rtx op, HOST_WIDE_INT } } else - { - type = TREE_TYPE (decl); + gcc_unreachable (); - dalign = TYPE_ALIGN (type); - if (CONSTANT_CLASS_P (decl)) - dalign = CONSTANT_ALIGNMENT (decl, dalign); - else - dalign = DATA_ALIGNMENT (decl, dalign); - - if (dsize == 0) - { - /* BLKmode, check the entire object. */ - if (TREE_CODE (decl) == STRING_CST) - dsize = TREE_STRING_LENGTH (decl); - else if (TYPE_SIZE_UNIT (type) - && tree_fits_uhwi_p (TYPE_SIZE_UNIT (type))) - dsize = tree_to_uhwi (TYPE_SIZE_UNIT (type)); - else - return false; - if (dsize > 32768) - return false; - - return dalign / BITS_PER_UNIT >= dsize; - } - } - /* Find how many bits of the alignment we know for this access. */ mask = dalign / BITS_PER_UNIT - 1; lsb = offset & -offset; -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RS6000] pr65810, powerpc64 alignment of r2 2015-04-20 13:53 [RS6000] pr65810, powerpc64 alignment of r2 Alan Modra @ 2015-04-20 14:27 ` Jakub Jelinek 2015-04-20 17:11 ` David Edelsohn 1 sibling, 0 replies; 6+ messages in thread From: Jakub Jelinek @ 2015-04-20 14:27 UTC (permalink / raw) To: gcc-patches, David Edelsohn On Mon, Apr 20, 2015 at 11:23:17PM +0930, Alan Modra wrote: > This fixes a thinko in offsettable_ok_by_alignment. It's not the > absolute placement that matters, but the toc-pointer relative offset. > So alignment of r2 also needs to be taken into account. > > Bootstrapped and regression tested powerpc64-linux. OK for mainline > and gcc-5 branch? Without the dead code removal for the branch.. Please wait until 5.1 is released on the gcc-5 branch. Jakub ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RS6000] pr65810, powerpc64 alignment of r2 2015-04-20 13:53 [RS6000] pr65810, powerpc64 alignment of r2 Alan Modra 2015-04-20 14:27 ` Jakub Jelinek @ 2015-04-20 17:11 ` David Edelsohn 2015-04-20 22:32 ` Alan Modra 1 sibling, 1 reply; 6+ messages in thread From: David Edelsohn @ 2015-04-20 17:11 UTC (permalink / raw) To: GCC Patches, Alan Modra On Mon, Apr 20, 2015 at 9:53 AM, Alan Modra <amodra@gmail.com> wrote: > This fixes a thinko in offsettable_ok_by_alignment. It's not the > absolute placement that matters, but the toc-pointer relative offset. > So alignment of r2 also needs to be taken into account. > > Bootstrapped and regression tested powerpc64-linux. OK for mainline > and gcc-5 branch? Without the dead code removal for the branch.. See question below. > > I also have a linker fix to align the toc pointer and gcc configury > changes to supply POWERPC64_TOC_POINTER_ALIGNMENT in the works. > > PR target/65810 > * config/rs6000/rs6000.c (POWERPC64_TOC_POINTER_ALIGNMENT): Define. > (offsettable_ok_by_alignment): Return false if size exceeds > POWERPC64_TOC_POINTER_ALIGNMENT. Replace dead code with assertion. > > Index: gcc/config/rs6000/rs6000.c > =================================================================== > --- gcc/config/rs6000/rs6000.c (revision 222227) > +++ gcc/config/rs6000/rs6000.c (working copy) > @@ -6497,13 +6497,21 @@ virtual_stack_registers_memory_p (rtx op) > } > > /* Return true if a MODE sized memory accesses to OP plus OFFSET > - is known to not straddle a 32k boundary. */ > + is known to not straddle a 32k boundary. This function is used > + to determine whether -mcmodel=medium code can use TOC pointer > + relative addressing for OP. This means the alignment of the TOC > + pointer must also be taken into account, and unfortunately that is > + only 8 bytes. */ > > +#ifndef POWERPC64_TOC_POINTER_ALIGNMENT > +#define POWERPC64_TOC_POINTER_ALIGNMENT 8 > +#endif > + > static bool > offsettable_ok_by_alignment (rtx op, HOST_WIDE_INT offset, > machine_mode mode) > { > - tree decl, type; > + tree decl; > unsigned HOST_WIDE_INT dsize, dalign, lsb, mask; > > if (GET_CODE (op) != SYMBOL_REF) > @@ -6510,6 +6518,8 @@ offsettable_ok_by_alignment (rtx op, HOST_WIDE_INT > return false; > > dsize = GET_MODE_SIZE (mode); > + if (dsize > POWERPC64_TOC_POINTER_ALIGNMENT) > + return false; Why do you immediately fail if the mode size is greater than the alignment? You may want to sprinkle some comments that this is assuming natural alignment (which is correct for the current ABI). But the DECL could be declared with less alignment. VMX needs this sort of restriction, but VSX would function okay. Or, does the generated code assume that the offsets from the TOC are units of the DECL size and not the TOC size? Thanks, David > decl = SYMBOL_REF_DECL (op); > if (!decl) > { > @@ -6553,6 +6563,8 @@ offsettable_ok_by_alignment (rtx op, HOST_WIDE_INT > return false; > > dsize = tree_to_uhwi (DECL_SIZE_UNIT (decl)); > + if (dsize > POWERPC64_TOC_POINTER_ALIGNMENT) > + return false; > if (dsize > 32768) > return false; > > @@ -6560,32 +6572,8 @@ offsettable_ok_by_alignment (rtx op, HOST_WIDE_INT > } > } > else > - { > - type = TREE_TYPE (decl); > + gcc_unreachable (); > > - dalign = TYPE_ALIGN (type); > - if (CONSTANT_CLASS_P (decl)) > - dalign = CONSTANT_ALIGNMENT (decl, dalign); > - else > - dalign = DATA_ALIGNMENT (decl, dalign); > - > - if (dsize == 0) > - { > - /* BLKmode, check the entire object. */ > - if (TREE_CODE (decl) == STRING_CST) > - dsize = TREE_STRING_LENGTH (decl); > - else if (TYPE_SIZE_UNIT (type) > - && tree_fits_uhwi_p (TYPE_SIZE_UNIT (type))) > - dsize = tree_to_uhwi (TYPE_SIZE_UNIT (type)); > - else > - return false; > - if (dsize > 32768) > - return false; > - > - return dalign / BITS_PER_UNIT >= dsize; > - } > - } > - > /* Find how many bits of the alignment we know for this access. */ > mask = dalign / BITS_PER_UNIT - 1; > lsb = offset & -offset; > > -- > Alan Modra > Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RS6000] pr65810, powerpc64 alignment of r2 2015-04-20 17:11 ` David Edelsohn @ 2015-04-20 22:32 ` Alan Modra 2015-04-23 11:24 ` Alan Modra 0 siblings, 1 reply; 6+ messages in thread From: Alan Modra @ 2015-04-20 22:32 UTC (permalink / raw) To: David Edelsohn; +Cc: GCC Patches On Mon, Apr 20, 2015 at 01:11:41PM -0400, David Edelsohn wrote: > > @@ -6510,6 +6518,8 @@ offsettable_ok_by_alignment (rtx op, HOST_WIDE_INT > > return false; > > > > dsize = GET_MODE_SIZE (mode); > > + if (dsize > POWERPC64_TOC_POINTER_ALIGNMENT) > > + return false; > > Why do you immediately fail if the mode size is greater than the alignment? The function is testing whether the address is offsettable within mode. To access components of the memory addressed, we might need to tack on an additional offset up to dsize-1 to the toc-relative offset. If the additional offset exceeds the known minimum toc pointer alignment then we might go over a n*64k+32k boundary. See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65810#c4 Now, if you were really asking "Why didn't you defer this test and instead take the lesser of the memory alignment and the toc pointer alignment in order to cover > But the DECL could be declared with less alignment. this case?", then the answer would be that I'm a dummy. I woke up this morning with a nagging feeling that the patch wasn't quite correct. So, given that Jakub has nixed the patch for 5.1, I may as well not rush this change. I'll spend some time first testing the linker fix and submit a better solution that has a configure test for POWERPC64_TOC_POINTER_ALIGNMENT. > VMX needs this sort of restriction, but VSX would function okay. > > Or, does the generated code assume that the offsets from the TOC are > units of the DECL size and not the TOC size? I'm not sure what you're asking here. Offsettable means we have a D-form instruction. As in the PR addis r9,r2,-2 lfd f24,32760(r9) lfd f25,-32768(r9) is bad. The patch I submitted changes this to addis r9,r2,-2 addi r9,r9,32760 lfd f24,0(r9) lfd f25,8(r9) -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RS6000] pr65810, powerpc64 alignment of r2 2015-04-20 22:32 ` Alan Modra @ 2015-04-23 11:24 ` Alan Modra 2015-04-27 21:11 ` David Edelsohn 0 siblings, 1 reply; 6+ messages in thread From: Alan Modra @ 2015-04-23 11:24 UTC (permalink / raw) To: David Edelsohn, GCC Patches Revised patch, supporting linker that aligns the toc base. This fixes a thinko in offsettable_ok_by_alignment. It's not the absolute placement that matters, but the toc-pointer relative offset. So alignment of r2 also needs to be taken into account. Changing offsettable_ok_by_alignment has a ripple effect into the 'm' constraint so we also need to ensure rs6000_legitimize_reload_address does not create invalid toc-relative addresses. As found by gcc.dg/torture/builtin-math-2.c -Os. That's the reason for the use_toc_relative_ref change. I hope the size check along with reg_offset_p is sufficient here. It seems so, but it's difficult to be certain due to how hard it is to get just the right combination of reload conditions to trigger. Bootstrapped and regression tested powerpc64-linux and powerpc64le-linux, both with a new and old linker. OK for mainline? PR target/65810 * config/rs6000/rs6000.c (POWERPC64_TOC_POINTER_ALIGNMENT): Define. (offsettable_ok_by_alignment): Use minimum of decl and toc pointer alignment. Replace dead code with assertion. (use_toc_relative_ref): Add mode arg. Return false in -mcmodel=medium case if size exceeds toc pointer alignment. (rs6000_legitimize_reload_address): Update use_toc_relative_ref call. (rs6000_emit_move): Likewise. * configure.ac: Add linker toc pointer alignment check. * configure: Regenerate. * config.in: Regenerate. Index: gcc/config/rs6000/rs6000.c =================================================================== --- gcc/config/rs6000/rs6000.c (revision 222352) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -6497,13 +6497,21 @@ virtual_stack_registers_memory_p (rtx op) } /* Return true if a MODE sized memory accesses to OP plus OFFSET - is known to not straddle a 32k boundary. */ + is known to not straddle a 32k boundary. This function is used + to determine whether -mcmodel=medium code can use TOC pointer + relative addressing for OP. This means the alignment of the TOC + pointer must also be taken into account, and unfortunately that is + only 8 bytes. */ +#ifndef POWERPC64_TOC_POINTER_ALIGNMENT +#define POWERPC64_TOC_POINTER_ALIGNMENT 8 +#endif + static bool offsettable_ok_by_alignment (rtx op, HOST_WIDE_INT offset, machine_mode mode) { - tree decl, type; + tree decl; unsigned HOST_WIDE_INT dsize, dalign, lsb, mask; if (GET_CODE (op) != SYMBOL_REF) @@ -6556,38 +6564,20 @@ offsettable_ok_by_alignment (rtx op, HOST_WIDE_INT if (dsize > 32768) return false; - return dalign / BITS_PER_UNIT >= dsize; + dalign /= BITS_PER_UNIT; + if (dalign > POWERPC64_TOC_POINTER_ALIGNMENT) + dalign = POWERPC64_TOC_POINTER_ALIGNMENT; + return dalign >= dsize; } } else - { - type = TREE_TYPE (decl); + gcc_unreachable (); - dalign = TYPE_ALIGN (type); - if (CONSTANT_CLASS_P (decl)) - dalign = CONSTANT_ALIGNMENT (decl, dalign); - else - dalign = DATA_ALIGNMENT (decl, dalign); - - if (dsize == 0) - { - /* BLKmode, check the entire object. */ - if (TREE_CODE (decl) == STRING_CST) - dsize = TREE_STRING_LENGTH (decl); - else if (TYPE_SIZE_UNIT (type) - && tree_fits_uhwi_p (TYPE_SIZE_UNIT (type))) - dsize = tree_to_uhwi (TYPE_SIZE_UNIT (type)); - else - return false; - if (dsize > 32768) - return false; - - return dalign / BITS_PER_UNIT >= dsize; - } - } - /* Find how many bits of the alignment we know for this access. */ - mask = dalign / BITS_PER_UNIT - 1; + dalign /= BITS_PER_UNIT; + if (dalign > POWERPC64_TOC_POINTER_ALIGNMENT) + dalign = POWERPC64_TOC_POINTER_ALIGNMENT; + mask = dalign - 1; lsb = offset & -offset; mask &= lsb - 1; dalign = mask + 1; @@ -7526,13 +7516,14 @@ rs6000_cannot_force_const_mem (machine_mode mode A can be addressed relative to the toc pointer. */ static bool -use_toc_relative_ref (rtx sym) +use_toc_relative_ref (rtx sym, machine_mode mode) { return ((constant_pool_expr_p (sym) && ASM_OUTPUT_SPECIAL_POOL_ENTRY_P (get_pool_constant (sym), get_pool_mode (sym))) || (TARGET_CMODEL == CMODEL_MEDIUM - && SYMBOL_REF_LOCAL_P (sym))); + && SYMBOL_REF_LOCAL_P (sym) + && GET_MODE_SIZE (mode) <= POWERPC64_TOC_POINTER_ALIGNMENT)); } /* Our implementation of LEGITIMIZE_RELOAD_ADDRESS. Returns a value to @@ -7737,7 +7728,7 @@ rs6000_legitimize_reload_address (rtx x, machine_m if (TARGET_TOC && reg_offset_p && GET_CODE (x) == SYMBOL_REF - && use_toc_relative_ref (x)) + && use_toc_relative_ref (x, mode)) { x = create_TOC_reference (x, NULL_RTX); if (TARGET_CMODEL != CMODEL_SMALL) @@ -8810,7 +8801,7 @@ rs6000_emit_move (rtx dest, rtx source, machine_mo reference to it. */ if (TARGET_TOC && GET_CODE (operands[1]) == SYMBOL_REF - && use_toc_relative_ref (operands[1])) + && use_toc_relative_ref (operands[1], mode)) operands[1] = create_TOC_reference (operands[1], operands[0]); else if (mode == Pmode && CONSTANT_P (operands[1]) Index: gcc/configure.ac =================================================================== --- gcc/configure.ac (revision 222352) +++ gcc/configure.ac (working copy) @@ -5087,6 +5087,31 @@ EOF AC_DEFINE(HAVE_LD_LARGE_TOC, 1, [Define if your PowerPC64 linker supports a large TOC.]) fi + + AC_CACHE_CHECK(linker toc pointer alignment, + gcc_cv_ld_toc_align, + [if test x$gcc_cv_as != x -a x$gcc_cv_ld != x -a x$gcc_cv_nm != x; then + cat > conftest.s <<EOF + .global _start + .text +_start: + addis 9,2,x@got@ha + .section .data.rel.ro,"aw",@progbits + .p2align 16 + .space 32768 +x: .quad .TOC. +EOF + if $gcc_cv_as -a64 -o conftest.o conftest.s > /dev/null 2>&1 \ + && $gcc_cv_ld $emul_name -o conftest conftest.o > /dev/null 2>&1; then + gcc_cv_ld_toc_align=`$gcc_cv_nm conftest | ${AWK} '/\.TOC\./ { match ($0, "0[[[:xdigit:]]]*", a); print strtonum ("0x" substr(a[[0]], length(a[[0]])-3)) }'` + fi + rm -f conftest conftest.o conftest.s + fi + ]) + if test -n "$gcc_cv_ld_toc_align" && test $gcc_cv_ld_toc_align -gt 8; then + AC_DEFINE_UNQUOTED(POWERPC64_TOC_POINTER_ALIGNMENT, $gcc_cv_ld_toc_align, + [Define to .TOC. alignment forced by your linker.]) + fi ;; esac -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RS6000] pr65810, powerpc64 alignment of r2 2015-04-23 11:24 ` Alan Modra @ 2015-04-27 21:11 ` David Edelsohn 0 siblings, 0 replies; 6+ messages in thread From: David Edelsohn @ 2015-04-27 21:11 UTC (permalink / raw) To: GCC Patches, Alan Modra On Thu, Apr 23, 2015 at 7:24 AM, Alan Modra <amodra@gmail.com> wrote: > Revised patch, supporting linker that aligns the toc base. > > This fixes a thinko in offsettable_ok_by_alignment. It's not the > absolute placement that matters, but the toc-pointer relative offset. > So alignment of r2 also needs to be taken into account. > > Changing offsettable_ok_by_alignment has a ripple effect into the 'm' > constraint so we also need to ensure rs6000_legitimize_reload_address > does not create invalid toc-relative addresses. As found by > gcc.dg/torture/builtin-math-2.c -Os. That's the reason for the > use_toc_relative_ref change. I hope the size check along with > reg_offset_p is sufficient here. It seems so, but it's difficult to > be certain due to how hard it is to get just the right combination of > reload conditions to trigger. > > Bootstrapped and regression tested powerpc64-linux and > powerpc64le-linux, both with a new and old linker. OK for mainline? > > PR target/65810 > * config/rs6000/rs6000.c (POWERPC64_TOC_POINTER_ALIGNMENT): Define. > (offsettable_ok_by_alignment): Use minimum of decl and toc > pointer alignment. Replace dead code with assertion. > (use_toc_relative_ref): Add mode arg. Return false in -mcmodel=medium > case if size exceeds toc pointer alignment. > (rs6000_legitimize_reload_address): Update use_toc_relative_ref call. > (rs6000_emit_move): Likewise. > * configure.ac: Add linker toc pointer alignment check. > * configure: Regenerate. > * config.in: Regenerate. Okay. Thanks, David ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-04-27 21:11 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-04-20 13:53 [RS6000] pr65810, powerpc64 alignment of r2 Alan Modra 2015-04-20 14:27 ` Jakub Jelinek 2015-04-20 17:11 ` David Edelsohn 2015-04-20 22:32 ` Alan Modra 2015-04-23 11:24 ` Alan Modra 2015-04-27 21:11 ` David Edelsohn
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).