Re: [PATCH v2, rs6000] Implement 32- and 64-bit BE handling for -mno-speculate-indirect-jumps
On Jan 19, 2018, at 12:32 PM, David Edelsohn wrote: > > This patch is incorrect for AIX. Which also means that the backport > to GCC 7 branch is incorrect for AIX and must be corrected before the > release. > > AIX assembler does not accept "." (period) as the current address. > > "b ." is incorrect. And testing for "b ." is incorrect. I am going > to try testing with "$" in trunk. > > GCC 7.3 must be re-spun. Thanks, David, patch in progress. FYI, I missed the 7.3 RC deadline, so this will only get picked up on a respin anyway. So I should have the fix in place before that happens. Bill > > Thanks, David > > On Tue, Jan 16, 2018 at 9:08 PM, Bill Schmidt > wrote: >> Hi, >> >> This patch supercedes and extends >> https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01479.html, >> adding the remaining big-endian support for -mno-speculate-indirect-jumps. >> This includes 32-bit support for indirect calls and sibling calls, and >> 64-bit support for indirect calls. The endian-neutral switch handling has >> already been committed. >> >> Using -m32 -O2 on safe-indirect-jumps-1.c results in a test for a sibling >> call, so this has been added as safe-indirect-jumps-8.c. Also, >> safe-indirect-jumps-7.c adds a variant that will not generate a sibling >> call for -m32, so we still get indirect call coverage. >> >> Bootstrapped and tested on powerpc64-linux-gnu and powerpc64le-linux-gnu >> with no regressions. Is this okay for trunk? >> >> Thanks, >> Bill >> >> >> [gcc] >> >> 2018-01-16 Bill Schmidt >> >>* config/rs6000/rs6000.md (*call_indirect_nonlocal_sysv): >>Generate different code for -mno-speculate-indirect-jumps. >>(*call_value_indirect_nonlocal_sysv): Likewise. >>(*call_indirect_aix): Disable for >>-mno-speculate-indirect-jumps. >>(*call_indirect_aix_nospec): New define_insn. >>(*call_value_indirect_aix): Disable for >>-mno-speculate-indirect-jumps. >>(*call_value_indirect_aix_nospec): New define_insn. >>(*sibcall_nonlocal_sysv): Generate different code for >>-mno-speculate-indirect-jumps. >>(*sibcall_value_nonlocal_sysv): Likewise. >> >> [gcc/testsuite] >> >> 2018-01-16 Bill Schmidt >> >>* gcc.target/powerpc/safe-indirect-jump-1.c: Remove endian >>restriction, but still restrict to 64-bit. >>* gcc.target/powerpc/safe-indirect-jump-7.c: New file. >>* gcc.target/powerpc/safe-indirect-jump-8.c: New file. >> >> >> Index: gcc/config/rs6000/rs6000.md >> === >> --- gcc/config/rs6000/rs6000.md (revision 256753) >> +++ gcc/config/rs6000/rs6000.md (working copy) >> @@ -10453,10 +10453,35 @@ >> else if (INTVAL (operands[2]) & CALL_V4_CLEAR_FP_ARGS) >> output_asm_insn ("creqv 6,6,6", operands); >> >> - return "b%T0l"; >> + if (rs6000_speculate_indirect_jumps >> + || which_alternative == 1 || which_alternative == 3) >> +return "b%T0l"; >> + else >> +return "crset eq\;beq%T0l-"; >> } >> [(set_attr "type" "jmpreg,jmpreg,jmpreg,jmpreg") >> - (set_attr "length" "4,4,8,8")]) >> + (set (attr "length") >> + (cond [(and (eq (symbol_ref "which_alternative") (const_int 0)) >> + (eq (symbol_ref "rs6000_speculate_indirect_jumps") >> + (const_int 1))) >> + (const_string "4") >> + (and (eq (symbol_ref "which_alternative") (const_int 0)) >> + (eq (symbol_ref "rs6000_speculate_indirect_jumps") >> + (const_int 0))) >> + (const_string "8") >> + (eq (symbol_ref "which_alternative") (const_int 1)) >> + (const_string "4") >> + (and (eq (symbol_ref "which_alternative") (const_int 2)) >> + (eq (symbol_ref "rs6000_speculate_indirect_jumps") >> + (const_int 1))) >> + (const_string "8") >> + (and (eq (symbol_ref "which_alternative") (const_int 2)) >> + (eq (symbol_ref "rs6000_speculate_indirect_jumps") >> + (const_int 0))) >> + (const_string "12") >> + (eq (symbol_ref "which_alternative") (const_int 3)) >> + (const_string "8")] >> + (const_string "4")))]) >> >> (define_insn_and_split "*call_nonlocal_sysv" >> [(call (mem:SI (match_operand:P 0 "symbol_ref_operand" "s,s")) >> @@ -10541,10 +10566,35 @@ >> else if (INTVAL (operands[3]) & CALL_V4_CLEAR_FP_ARGS) >> output_asm_insn ("creqv 6,6,6", operands); >> >> - return "b%T1l"; >> + if (rs6000_speculate_indirect_jumps >> + || which_alternative == 1 || which_alternative == 3) >> +return "b%T1l"; >> + else >> +return "crset eq\;beq%T1l-"; >> } >> [(set_attr "type" "jmpreg,jmpreg,jmpreg,jmpreg") >> - (set_attr "length" "4,4,8,8")]) >> + (set (attr "length") >> + (cond [(and (eq (symb
Re: [PATCH v2, rs6000] Implement 32- and 64-bit BE handling for -mno-speculate-indirect-jumps
This patch is incorrect for AIX. Which also means that the backport to GCC 7 branch is incorrect for AIX and must be corrected before the release. AIX assembler does not accept "." (period) as the current address. "b ." is incorrect. And testing for "b ." is incorrect. I am going to try testing with "$" in trunk. GCC 7.3 must be re-spun. Thanks, David On Tue, Jan 16, 2018 at 9:08 PM, Bill Schmidt wrote: > Hi, > > This patch supercedes and extends > https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01479.html, > adding the remaining big-endian support for -mno-speculate-indirect-jumps. > This includes 32-bit support for indirect calls and sibling calls, and > 64-bit support for indirect calls. The endian-neutral switch handling has > already been committed. > > Using -m32 -O2 on safe-indirect-jumps-1.c results in a test for a sibling > call, so this has been added as safe-indirect-jumps-8.c. Also, > safe-indirect-jumps-7.c adds a variant that will not generate a sibling > call for -m32, so we still get indirect call coverage. > > Bootstrapped and tested on powerpc64-linux-gnu and powerpc64le-linux-gnu > with no regressions. Is this okay for trunk? > > Thanks, > Bill > > > [gcc] > > 2018-01-16 Bill Schmidt > > * config/rs6000/rs6000.md (*call_indirect_nonlocal_sysv): > Generate different code for -mno-speculate-indirect-jumps. > (*call_value_indirect_nonlocal_sysv): Likewise. > (*call_indirect_aix): Disable for > -mno-speculate-indirect-jumps. > (*call_indirect_aix_nospec): New define_insn. > (*call_value_indirect_aix): Disable for > -mno-speculate-indirect-jumps. > (*call_value_indirect_aix_nospec): New define_insn. > (*sibcall_nonlocal_sysv): Generate different code for > -mno-speculate-indirect-jumps. > (*sibcall_value_nonlocal_sysv): Likewise. > > [gcc/testsuite] > > 2018-01-16 Bill Schmidt > > * gcc.target/powerpc/safe-indirect-jump-1.c: Remove endian > restriction, but still restrict to 64-bit. > * gcc.target/powerpc/safe-indirect-jump-7.c: New file. > * gcc.target/powerpc/safe-indirect-jump-8.c: New file. > > > Index: gcc/config/rs6000/rs6000.md > === > --- gcc/config/rs6000/rs6000.md (revision 256753) > +++ gcc/config/rs6000/rs6000.md (working copy) > @@ -10453,10 +10453,35 @@ >else if (INTVAL (operands[2]) & CALL_V4_CLEAR_FP_ARGS) > output_asm_insn ("creqv 6,6,6", operands); > > - return "b%T0l"; > + if (rs6000_speculate_indirect_jumps > + || which_alternative == 1 || which_alternative == 3) > +return "b%T0l"; > + else > +return "crset eq\;beq%T0l-"; > } >[(set_attr "type" "jmpreg,jmpreg,jmpreg,jmpreg") > - (set_attr "length" "4,4,8,8")]) > + (set (attr "length") > + (cond [(and (eq (symbol_ref "which_alternative") (const_int 0)) > + (eq (symbol_ref "rs6000_speculate_indirect_jumps") > + (const_int 1))) > + (const_string "4") > + (and (eq (symbol_ref "which_alternative") (const_int 0)) > + (eq (symbol_ref "rs6000_speculate_indirect_jumps") > + (const_int 0))) > + (const_string "8") > + (eq (symbol_ref "which_alternative") (const_int 1)) > + (const_string "4") > + (and (eq (symbol_ref "which_alternative") (const_int 2)) > + (eq (symbol_ref "rs6000_speculate_indirect_jumps") > + (const_int 1))) > + (const_string "8") > + (and (eq (symbol_ref "which_alternative") (const_int 2)) > + (eq (symbol_ref "rs6000_speculate_indirect_jumps") > + (const_int 0))) > + (const_string "12") > + (eq (symbol_ref "which_alternative") (const_int 3)) > + (const_string "8")] > + (const_string "4")))]) > > (define_insn_and_split "*call_nonlocal_sysv" >[(call (mem:SI (match_operand:P 0 "symbol_ref_operand" "s,s")) > @@ -10541,10 +10566,35 @@ >else if (INTVAL (operands[3]) & CALL_V4_CLEAR_FP_ARGS) > output_asm_insn ("creqv 6,6,6", operands); > > - return "b%T1l"; > + if (rs6000_speculate_indirect_jumps > + || which_alternative == 1 || which_alternative == 3) > +return "b%T1l"; > + else > +return "crset eq\;beq%T1l-"; > } >[(set_attr "type" "jmpreg,jmpreg,jmpreg,jmpreg") > - (set_attr "length" "4,4,8,8")]) > + (set (attr "length") > + (cond [(and (eq (symbol_ref "which_alternative") (const_int 0)) > + (eq (symbol_ref "rs6000_speculate_indirect_jumps") > + (const_int 1))) > + (const_string "4") > + (and (eq (symbol_ref "which_alternative") (const_int 0)) > + (eq (symbol_ref "rs6000_speculate_indirect_jumps") > +
Re: [PATCH v2, rs6000] Implement 32- and 64-bit BE handling for -mno-speculate-indirect-jumps
On Tue, Jan 16, 2018 at 08:08:57PM -0600, Bill Schmidt wrote: > This patch supercedes and extends > https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01479.html, > adding the remaining big-endian support for -mno-speculate-indirect-jumps. > This includes 32-bit support for indirect calls and sibling calls, and > 64-bit support for indirect calls. The endian-neutral switch handling has > already been committed. > > Using -m32 -O2 on safe-indirect-jumps-1.c results in a test for a sibling > call, so this has been added as safe-indirect-jumps-8.c. Also, > safe-indirect-jumps-7.c adds a variant that will not generate a sibling > call for -m32, so we still get indirect call coverage. > > Bootstrapped and tested on powerpc64-linux-gnu and powerpc64le-linux-gnu > with no regressions. Is this okay for trunk? Okay for trunk and backports. A few possible cleanups (okay with or without, you need to get this in soon): > - (set_attr "length" "4,4,8,8")]) > + (set (attr "length") > + (cond [(and (eq (symbol_ref "which_alternative") (const_int 0)) > + (eq (symbol_ref "rs6000_speculate_indirect_jumps") > + (const_int 1))) > + (const_string "4") You could leave out the "4" cases, it's the default. Might be easier to read. I'd use "ne 0" instead of "eq 1", but this will work I guess. > @@ -10909,7 +10982,13 @@ > output_asm_insn (\"creqv 6,6,6\", operands); > >if (which_alternative >= 2) > -return \"b%T0\"; > +{ > + if (rs6000_speculate_indirect_jumps) > + return \"b%T0\"; You can write the block as a block ({}) instead of as a string ("*{}") so you don't need all the backslashes. Thanks for all the work! Segher
[PATCH v2, rs6000] Implement 32- and 64-bit BE handling for -mno-speculate-indirect-jumps
Hi, This patch supercedes and extends https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01479.html, adding the remaining big-endian support for -mno-speculate-indirect-jumps. This includes 32-bit support for indirect calls and sibling calls, and 64-bit support for indirect calls. The endian-neutral switch handling has already been committed. Using -m32 -O2 on safe-indirect-jumps-1.c results in a test for a sibling call, so this has been added as safe-indirect-jumps-8.c. Also, safe-indirect-jumps-7.c adds a variant that will not generate a sibling call for -m32, so we still get indirect call coverage. Bootstrapped and tested on powerpc64-linux-gnu and powerpc64le-linux-gnu with no regressions. Is this okay for trunk? Thanks, Bill [gcc] 2018-01-16 Bill Schmidt * config/rs6000/rs6000.md (*call_indirect_nonlocal_sysv): Generate different code for -mno-speculate-indirect-jumps. (*call_value_indirect_nonlocal_sysv): Likewise. (*call_indirect_aix): Disable for -mno-speculate-indirect-jumps. (*call_indirect_aix_nospec): New define_insn. (*call_value_indirect_aix): Disable for -mno-speculate-indirect-jumps. (*call_value_indirect_aix_nospec): New define_insn. (*sibcall_nonlocal_sysv): Generate different code for -mno-speculate-indirect-jumps. (*sibcall_value_nonlocal_sysv): Likewise. [gcc/testsuite] 2018-01-16 Bill Schmidt * gcc.target/powerpc/safe-indirect-jump-1.c: Remove endian restriction, but still restrict to 64-bit. * gcc.target/powerpc/safe-indirect-jump-7.c: New file. * gcc.target/powerpc/safe-indirect-jump-8.c: New file. Index: gcc/config/rs6000/rs6000.md === --- gcc/config/rs6000/rs6000.md (revision 256753) +++ gcc/config/rs6000/rs6000.md (working copy) @@ -10453,10 +10453,35 @@ else if (INTVAL (operands[2]) & CALL_V4_CLEAR_FP_ARGS) output_asm_insn ("creqv 6,6,6", operands); - return "b%T0l"; + if (rs6000_speculate_indirect_jumps + || which_alternative == 1 || which_alternative == 3) +return "b%T0l"; + else +return "crset eq\;beq%T0l-"; } [(set_attr "type" "jmpreg,jmpreg,jmpreg,jmpreg") - (set_attr "length" "4,4,8,8")]) + (set (attr "length") + (cond [(and (eq (symbol_ref "which_alternative") (const_int 0)) + (eq (symbol_ref "rs6000_speculate_indirect_jumps") + (const_int 1))) + (const_string "4") + (and (eq (symbol_ref "which_alternative") (const_int 0)) + (eq (symbol_ref "rs6000_speculate_indirect_jumps") + (const_int 0))) + (const_string "8") + (eq (symbol_ref "which_alternative") (const_int 1)) + (const_string "4") + (and (eq (symbol_ref "which_alternative") (const_int 2)) + (eq (symbol_ref "rs6000_speculate_indirect_jumps") + (const_int 1))) + (const_string "8") + (and (eq (symbol_ref "which_alternative") (const_int 2)) + (eq (symbol_ref "rs6000_speculate_indirect_jumps") + (const_int 0))) + (const_string "12") + (eq (symbol_ref "which_alternative") (const_int 3)) + (const_string "8")] + (const_string "4")))]) (define_insn_and_split "*call_nonlocal_sysv" [(call (mem:SI (match_operand:P 0 "symbol_ref_operand" "s,s")) @@ -10541,10 +10566,35 @@ else if (INTVAL (operands[3]) & CALL_V4_CLEAR_FP_ARGS) output_asm_insn ("creqv 6,6,6", operands); - return "b%T1l"; + if (rs6000_speculate_indirect_jumps + || which_alternative == 1 || which_alternative == 3) +return "b%T1l"; + else +return "crset eq\;beq%T1l-"; } [(set_attr "type" "jmpreg,jmpreg,jmpreg,jmpreg") - (set_attr "length" "4,4,8,8")]) + (set (attr "length") + (cond [(and (eq (symbol_ref "which_alternative") (const_int 0)) + (eq (symbol_ref "rs6000_speculate_indirect_jumps") + (const_int 1))) + (const_string "4") + (and (eq (symbol_ref "which_alternative") (const_int 0)) + (eq (symbol_ref "rs6000_speculate_indirect_jumps") + (const_int 0))) + (const_string "8") + (eq (symbol_ref "which_alternative") (const_int 1)) + (const_string "4") + (and (eq (symbol_ref "which_alternative") (const_int 2)) + (eq (symbol_ref "rs6000_speculate_indirect_jumps") + (const_int 1))) + (const_string "8") + (and (eq (symbol_ref "which_alternative") (const_int 2)) + (eq (symbol_ref "rs6000_speculate_indirect_jumps") + (const_int 0))) + (const_string "12") +