| Message ID | 2c69adfc-b9db-687d-3a32-08f2efdde647@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC,LRA] WIP patch to fix one part of PR87507 | expand |
On 10/19/18 4:16 PM, Peter Bergner wrote: > Thoughts? I'll note that this does not fix the S390 bugs, since those seem > to be due to problems with early clobber operands and "matching" constraint > operands. I'm still working on that and hope to have something soon. [snip] > * lra-constraints.c (process_alt_operands): Abort on illegal hard > register usage. Prefer reloading non hard register operands. I stand corrected. Using this patch, plus Segher's combine patch, I am able to bootstrap s390x-linux. I'm running the test suite to see whether that looks clean as well. Maybe those s390 issues were related to combine pushing hard regs into patterns and we just weren't handling them well? Jeff, maybe once Segher commits his patch, can you give this patch a try on your testers? Peter
Hi Peter, On Fri, Oct 19, 2018 at 04:16:12PM -0500, Peter Bergner wrote: > In lra-constraints.c:process_alt_operands(), we notice that pseudo 92 is > assigned to x1 and that an early clobber operand is also assigned to x1, or > rather, that it uses x1 explicitly. This is enough to trigger reload(s), > but the problem is we end up trying to reload the early clobber operand > which has been forced into x1 via register asm assignment, instead of > pseudo 92 which conflicts with it. > Secondly, I don't think it's ever legal to reload an operand > that has been hard coded by the user to a hard register via a register asm > definition like in the test case above. Yup. > With that in mind, what do people think of the patch below? This fixes the > AARCH64 test case. However, it ICE's on the following test case: > > long foo (long arg) > { > register long var asm("x0"); > asm("bla %0 %1" : "+&r"(var) : "r"(arg)); > return var; > } (As an "unable to generate reloads for" fatal). > ...but that is due to a combine bug where combine replaces the use of > "arg"'s pseudo in the inline asm with the incoming argument reg x0 which > should be illegal. Ie, it's taking a valid inline asm and creating an > invalid one since the earlyclobber op and non-matching op have both > been forced to use the same hard reg. Segher has a combine patch to > stop that which he is going to submit (commit?) soon. Yup. It is quite a bit more generic: it prevents combine from combining a hard-not-fixed-reg-to-pseudo-copy with any instruction, not just with asm's. > With my patch, > we are also now able to catch the following user error, when before we > could not: > > bergner@pike:~/gcc/BUGS/PR87507/AARCH64$ cat ice4.i > long foo (void) > { > register long arg asm("x1"); > register long var asm("x1"); > asm ("bla %0 %1" : "=&r"(arg) : "r"(var)); > return arg; > } > bergner@pike:~/gcc/BUGS/PR87507/AARCH64$ .../xgcc -B.../gcc -O2 -march=armv8.1-a -S ice4.i > ice4.i: In function âfooâ: > ice4.i:7:1: error: unable to generate reloads for: > 7 | } > | ^ > (insn 5 6 10 2 (set (reg/v:DI 1 x1 [ arg ]) > (asm_operands:DI ("bla %0 %1") ("=&r") 0 [ > (reg/v:DI 1 x1 [ var ]) > ] > [ > (asm_input:DI ("r") ice4.i:5) > ] > [] ice4.i:5)) "ice4.i":5 -1 > (nil)) > during RTL pass: reload > ice4.i:7:1: internal compiler error: in process_alt_operands, at lra-constraints.c:2911 Nice! What did it do before? Oh, it reloaded things into other regs, generating incorrect code. Ouch! > I can't test > this on aarch64 or arm, other than knowing my aarch64 cross doesn't ICE on > the test case above. The compile farm has six aarch64 machines ;-) > --- gcc/lra-constraints.c (revision 264897) > +++ gcc/lra-constraints.c (working copy) > @@ -2904,18 +2904,29 @@ process_alt_operands (int only_alternati > if (first_conflict_j < 0) > first_conflict_j = j; > last_conflict_j = j; > + /* Both the earlyclobber operand and conflicting operand > + cannot both be hard registers. */ > + if (REGNO (operand_reg[i]) < FIRST_PSEUDO_REGISTER > + && operand_reg[j] != NULL_RTX > + && REGNO (operand_reg[j]) < FIRST_PSEUDO_REGISTER) > + fatal_insn ("unable to generate reloads for:", curr_insn); You could use HARD_REGISTER_P here, fwiw. Segher
On Fri, Oct 19, 2018 at 05:39:07PM -0500, Peter Bergner wrote: > On 10/19/18 4:16 PM, Peter Bergner wrote: > > Thoughts? I'll note that this does not fix the S390 bugs, since those seem > > to be due to problems with early clobber operands and "matching" constraint > > operands. I'm still working on that and hope to have something soon. > [snip] > > * lra-constraints.c (process_alt_operands): Abort on illegal hard > > register usage. Prefer reloading non hard register operands. > > I stand corrected. Using this patch, plus Segher's combine patch, I am > able to bootstrap s390x-linux. I'm running the test suite to see whether > that looks clean as well. Maybe those s390 issues were related to combine > pushing hard regs into patterns and we just weren't handling them well? The only such case where combine does something wrong (or that results in broken code, anyway) is the register vars with asm, AFAIK. It can of course make like harder for LRA/reload, but that shouldn't affect correctness. It would be interesting to hear about other cases. Segher
On 10/19/18 4:39 PM, Peter Bergner wrote: > On 10/19/18 4:16 PM, Peter Bergner wrote: >> Thoughts? I'll note that this does not fix the S390 bugs, since those seem >> to be due to problems with early clobber operands and "matching" constraint >> operands. I'm still working on that and hope to have something soon. > [snip] >> * lra-constraints.c (process_alt_operands): Abort on illegal hard >> register usage. Prefer reloading non hard register operands. > > I stand corrected. Using this patch, plus Segher's combine patch, I am > able to bootstrap s390x-linux. I'm running the test suite to see whether > that looks clean as well. Maybe those s390 issues were related to combine > pushing hard regs into patterns and we just weren't handling them well? > > Jeff, maybe once Segher commits his patch, can you give this patch a try > on your testers? Once committed to the trunk it's automatically picked up :-) In fact, commits to the trunk are triggers, though in reality there's *always* something changing from one day to the next in the various relevant repos (gcc, binutils, linux kernel, glibc & newlib). It's only testing patches that aren't on the trunk that require manual intervention. jeff
On 10/19/2018 05:16 PM, Peter Bergner wrote: > Vlad, Jeff and Segher, > > I think I have determined what is happening with the aarch64 test case that > is failing after my r264897 commit. It appears my patch is just exposing > an issue in lra-constraints.c:process_alt_operands() when processing an insn > with early clobber operands. Jeff & Segher, I have reverted all of my > previous patches we were making to lra-lives.c for the analysis below and > am just using straight trunk revision 264897 (ie, my last patch). ... > Secondly, I don't think it's ever legal to reload an operand > that has been hard coded by the user to a hard register via a register asm > definition like in the test case above. Yes, if a variable declared as an asm variable, the asm insn should use its hard register. That is what people expect and that is what the GCC documentation about asm says. In most cases, reload of such asm variable hard registers do not happen as they satisfy the constraints except for this complicated earlyclobber case you found. > With that in mind, what do people think of the patch below? This fixes the > AARCH64 test case. However, it ICE's on the following test case: > > long foo (long arg) > { > register long var asm("x0"); > asm("bla %0 %1" : "+&r"(var) : "r"(arg)); > return var; > } > > ...but that is due to a combine bug where combine replaces the use of > "arg"'s pseudo in the inline asm with the incoming argument reg x0 which > should be illegal. Ie, it's taking a valid inline asm and creating an > invalid one since the earlyclobber op and non-matching op have both > been forced to use the same hard reg. Segher has a combine patch to > stop that which he is going to submit (commit?) soon. With my patch, > we are also now able to catch the following user error, when before we > could not: I think usage of hard registers in RTL should be minimal. Any their propagation besides RA creates a risk for RA work and probably does not improve the final code. > > ... > > Thoughts? I'll note that this does not fix the S390 bugs, since those seem > to be due to problems with early clobber operands and "matching" constraint > operands. I'm still working on that and hope to have something soon. I think your initial patch triggered a bug in RA. Your patch looks a reasonable solution for me. But the patch touches a very sensitive code in LRA (I don't remember when a change in lra-constraints.c:process_alt_operands worked completely fine at the first iteration). So it would be nice to test it on many targets before committing. Peter, thank you for continuing your work on these RA issues. > I am currently bootstrapping this on s390x just to make sure I don't break > anything there, before debugging the existing s390x issue(s). I can't test > this on aarch64 or arm, other than knowing my aarch64 cross doesn't ICE on > the test case above. > > Peter > > * lra-constraints.c (process_alt_operands): Abort on illegal hard > register usage. Prefer reloading non hard register operands. > > Index: gcc/lra-constraints.c > =================================================================== > --- gcc/lra-constraints.c (revision 264897) > +++ gcc/lra-constraints.c (working copy) > @@ -2904,18 +2904,29 @@ process_alt_operands (int only_alternati > if (first_conflict_j < 0) > first_conflict_j = j; > last_conflict_j = j; > + /* Both the earlyclobber operand and conflicting operand > + cannot both be hard registers. */ > + if (REGNO (operand_reg[i]) < FIRST_PSEUDO_REGISTER > + && operand_reg[j] != NULL_RTX > + && REGNO (operand_reg[j]) < FIRST_PSEUDO_REGISTER) > + fatal_insn ("unable to generate reloads for:", curr_insn); > } > if (last_conflict_j < 0) > continue; > - /* If earlyclobber operand conflicts with another > - non-matching operand which is actually the same register > - as the earlyclobber operand, it is better to reload the > - another operand as an operand matching the earlyclobber > - operand can be also the same. */ > - if (first_conflict_j == last_conflict_j > - && operand_reg[last_conflict_j] != NULL_RTX > - && ! curr_alt_match_win[last_conflict_j] > - && REGNO (operand_reg[i]) == REGNO (operand_reg[last_conflict_j])) > + > + /* If an earlyclobber operand conflicts with another non-matching > + operand (ie, they have been assigned the same hard register), > + then it is better to reload the other operand, as there may > + exist yet another operand with a matching constraint associated > + with the earlyclobber operand. However, if one of the operands > + is an explicit use of a hard register, then we must reload the > + other non-hard register operand. */ > + if (REGNO (operand_reg[i]) < FIRST_PSEUDO_REGISTER > + || (operand_reg[last_conflict_j] != NULL_RTX > + && REGNO (operand_reg[last_conflict_j]) > + >= FIRST_PSEUDO_REGISTER > + && first_conflict_j == last_conflict_j > + && ! curr_alt_match_win[last_conflict_j])) > { > curr_alt_win[last_conflict_j] = false; > curr_alt_dont_inherit_ops[curr_alt_dont_inherit_ops_num++] >
On 10/22/18 3:58 PM, Jeff Law wrote: > On 10/19/18 4:39 PM, Peter Bergner wrote: >> Jeff, maybe once Segher commits his patch, can you give this patch a try >> on your testers? > Once committed to the trunk it's automatically picked up :-) In fact, > commits to the trunk are triggers, though in reality there's *always* > something changing from one day to the next in the various relevant > repos (gcc, binutils, linux kernel, glibc & newlib). > > It's only testing patches that aren't on the trunk that require manual > intervention. I meant could you give *my* patch a try once Segher's combine patch hit trunk. It didn't really make sense to try my patch before his patch hit trunk, since my patch was catching some illegal insn constraint usage. That said, here is an updated patch I'd like you to try now that Segher's patch is on trunk. It's basically the same patch as before, but using HARD_REGISTER_P suggested by Segher and another change that catches more illegal constraint usage, this time matching constraints that don't match, but where the user has incorrectly forced register usage to different hard regs. We used to incorrectly reload one of the hard coded operands and go on our merry way. Now we throw an error: bergner@pike:~/gcc/BUGS/PR87507$ cat ice6.i long foo (void) { register long arg asm("r3"); register long var asm("r4"); asm ("bla %0 %1" : "=r"(arg) : "0"(var)); return arg; } bergner@pike:~/gcc/BUGS/PR87507$ /home/bergner/gcc/build/gcc-fsf-mainline-pr87507-debug/gcc/xgcc -B/home/bergner/gcc/build/gcc-fsf-mainline-pr87507-debug/gcc -O2 -S ice6.i ice6.i: In function ‘foo’: ice6.i:7:1: error: unable to fixup asm constraints for: 7 | } | ^ (insn 5 2 11 2 (parallel [ (set (reg/v:DI 3 3 [ arg ]) (asm_operands:DI ("bla %0 %1") ("=r") 0 [ (reg/v:DI 4 4 [ var ]) ] [ (asm_input:DI ("0") ice6.i:5) ] [] ice6.i:5)) (clobber (reg:SI 76 ca)) ]) "ice6.i":5:3 -1 (expr_list:REG_DEAD (reg/v:DI 4 4 [ var ]) (expr_list:REG_UNUSED (reg:SI 76 ca) (nil)))) during RTL pass: asmcons ice6.i:7:1: internal compiler error: in match_asm_constraints_1, at function.c:6461 I'm currently bootstrapping and regtesting this on powerpc64le-linux, x86_64-linux and s390x-linux. If you could throw it on your testers to test the other arches, that would be great! Peter * function.c (match_asm_constraints_1): Catch illegal inline asm constraint usage. * lra-constraints.c (process_alt_operands): Abort on illegal hard register usage. Prefer reloading non hard register operands. Index: gcc/function.c =================================================================== --- gcc/function.c (revision 265399) +++ gcc/function.c (working copy) @@ -6453,6 +6453,13 @@ match_asm_constraints_1 (rtx_insn *insn, || !general_operand (input, GET_MODE (output))) continue; + /* If we have a matching constraint and both operands are hard registers, + then they must be the same hard register. */ + if (HARD_REGISTER_P (output) + && HARD_REGISTER_P (input) + && REGNO (output) != REGNO (input)) + fatal_insn ("unable to fixup asm constraints for:", insn); + /* We can't do anything if the output is also used as input, as we're going to overwrite it. */ for (j = 0; j < ninputs; j++) Index: gcc/lra-constraints.c =================================================================== --- gcc/lra-constraints.c (revision 265399) +++ gcc/lra-constraints.c (working copy) @@ -2146,9 +2146,15 @@ process_alt_operands (int only_alternati } else { - /* Operands don't match. Both operands must - allow a reload register, otherwise we - cannot make them match. */ + /* Operands don't match. Make sure the two operands + are not two different explicit hard registers. */ + if (HARD_REGISTER_P (*curr_id->operand_loc[nop]) + && HARD_REGISTER_P (*curr_id->operand_loc[m])) + fatal_insn ("unable to generate reloads for:", + curr_insn); + + /* Both operands must allow a reload register, + otherwise we cannot make them match. */ if (curr_alt[m] == NO_REGS) break; /* Retroactively mark the operand we had to @@ -2910,18 +2916,28 @@ process_alt_operands (int only_alternati if (first_conflict_j < 0) first_conflict_j = j; last_conflict_j = j; + /* Both the earlyclobber operand and conflicting operand + cannot both be hard registers. */ + if (HARD_REGISTER_P (operand_reg[i]) + && operand_reg[j] != NULL_RTX + && HARD_REGISTER_P (operand_reg[j])) + fatal_insn ("unable to generate reloads for:", curr_insn); } if (last_conflict_j < 0) continue; - /* If earlyclobber operand conflicts with another - non-matching operand which is actually the same register - as the earlyclobber operand, it is better to reload the - another operand as an operand matching the earlyclobber - operand can be also the same. */ - if (first_conflict_j == last_conflict_j - && operand_reg[last_conflict_j] != NULL_RTX - && ! curr_alt_match_win[last_conflict_j] - && REGNO (operand_reg[i]) == REGNO (operand_reg[last_conflict_j])) + + /* If an earlyclobber operand conflicts with another non-matching + operand (ie, they have been assigned the same hard register), + then it is better to reload the other operand, as there may + exist yet another operand with a matching constraint associated + with the earlyclobber operand. However, if one of the operands + is an explicit use of a hard register, then we must reload the + other non-hard register operand. */ + if (HARD_REGISTER_P (operand_reg[i]) + || (first_conflict_j == last_conflict_j + && operand_reg[last_conflict_j] != NULL_RTX + && !curr_alt_match_win[last_conflict_j] + && !HARD_REGISTER_P (operand_reg[last_conflict_j]))) { curr_alt_win[last_conflict_j] = false; curr_alt_dont_inherit_ops[curr_alt_dont_inherit_ops_num++]
On 10/22/18 6:40 PM, Peter Bergner wrote: > On 10/22/18 3:58 PM, Jeff Law wrote: >> On 10/19/18 4:39 PM, Peter Bergner wrote: >>> Jeff, maybe once Segher commits his patch, can you give this patch a try >>> on your testers? >> Once committed to the trunk it's automatically picked up :-) In fact, >> commits to the trunk are triggers, though in reality there's *always* >> something changing from one day to the next in the various relevant >> repos (gcc, binutils, linux kernel, glibc & newlib). >> >> It's only testing patches that aren't on the trunk that require manual >> intervention. > > I meant could you give *my* patch a try once Segher's combine patch hit trunk. > It didn't really make sense to try my patch before his patch hit trunk, since > my patch was catching some illegal insn constraint usage. Bah, my bootstrap failed and I need to make a small change. Let me do that and verify my bootstraps get all the way thru before I give you the updated patch. Sorry. Peter
Hi peter, On Mon, Oct 22, 2018 at 06:40:58PM -0500, Peter Bergner wrote: > --- gcc/function.c (revision 265399) > +++ gcc/function.c (working copy) > @@ -6453,6 +6453,13 @@ match_asm_constraints_1 (rtx_insn *insn, > || !general_operand (input, GET_MODE (output))) > continue; > > + /* If we have a matching constraint and both operands are hard registers, > + then they must be the same hard register. */ > + if (HARD_REGISTER_P (output) > + && HARD_REGISTER_P (input) > + && REGNO (output) != REGNO (input)) You need to test for REG_P (input) before you can HARD_REGISTER_P (input) or REGNO (input). > + fatal_insn ("unable to fixup asm constraints for:", insn); "impossible constraints"? There are some more of those already. Or you could describe the actual problem even? > + /* Operands don't match. Make sure the two operands > + are not two different explicit hard registers. */ > + if (HARD_REGISTER_P (*curr_id->operand_loc[nop]) > + && HARD_REGISTER_P (*curr_id->operand_loc[m])) > + fatal_insn ("unable to generate reloads for:", > + curr_insn); Same here (and below) :-) Segher
On 10/22/18 7:20 PM, Segher Boessenkool wrote: > Hi peter, >> + /* If we have a matching constraint and both operands are hard registers, >> + then they must be the same hard register. */ >> + if (HARD_REGISTER_P (output) >> + && HARD_REGISTER_P (input) >> + && REGNO (output) != REGNO (input)) > > You need to test for REG_P (input) before you can HARD_REGISTER_P (input) > or REGNO (input). Yup, already done that. Just checking the s390 debugger which is seeing a SUBREG, so trying to decide whether I should be looking inside the SUBREG or not. Shouldn't be hard to do if it makes sense. >> + fatal_insn ("unable to fixup asm constraints for:", insn); > > "impossible constraints"? There are some more of those already. Or you > could describe the actual problem even? > >> + /* Operands don't match. Make sure the two operands >> + are not two different explicit hard registers. */ >> + if (HARD_REGISTER_P (*curr_id->operand_loc[nop]) >> + && HARD_REGISTER_P (*curr_id->operand_loc[m])) >> + fatal_insn ("unable to generate reloads for:", >> + curr_insn); > > Same here (and below) :-) Yeah, I like that. Thanks. Peter
On 10/22/18 6:20 PM, Segher Boessenkool wrote: > Hi peter, > > On Mon, Oct 22, 2018 at 06:40:58PM -0500, Peter Bergner wrote: >> --- gcc/function.c (revision 265399) >> +++ gcc/function.c (working copy) >> @@ -6453,6 +6453,13 @@ match_asm_constraints_1 (rtx_insn *insn, >> || !general_operand (input, GET_MODE (output))) >> continue; >> >> + /* If we have a matching constraint and both operands are hard registers, >> + then they must be the same hard register. */ >> + if (HARD_REGISTER_P (output) >> + && HARD_REGISTER_P (input) >> + && REGNO (output) != REGNO (input)) > > You need to test for REG_P (input) before you can HARD_REGISTER_P (input) > or REGNO (input). One might argue that checking REG_P should be part of HARD_REGISTER_P. Jeff
On 10/22/18 6:45 PM, Peter Bergner wrote: > Bah, my bootstrap failed and I need to make a small change. Let me do that > and verify my bootstraps get all the way thru before I give you the updated > patch. Sorry. Ok, the following updated patch survives bootstrap and regtesting on powerpc64le-linux, x86_64-linux and s390x-linux with no regressions. Changes from the previous patch is that checking for illegal hard register usage in inline asm statements has been moved to expand time. Secondly, the lra constraints code now checks for both HARD_REGISTER_P and REG_USERVAR_P. This was because I was seeing combine forcing hard regs into a pattern (not from inline asm) which lra needed to spill to match constraints, which it should be able to do. Jeff, can you give this a try on your testers to see how it behaves on the other arches that were having problems? Peter PR rtl-optimization/87600 * cfgexpand.c (expand_asm_stmt): Catch illegal asm constraint usage. * lra-constraints.c (process_alt_operands): Skip illegal hard register usage. Prefer reloading non hard register operands. Index: gcc/cfgexpand.c =================================================================== --- gcc/cfgexpand.c (revision 265402) +++ gcc/cfgexpand.c (working copy) @@ -3010,6 +3010,55 @@ expand_asm_stmt (gasm *stmt) &allows_mem, &allows_reg, &is_inout)) return; + /* If the output is a hard register, verify it doesn't conflict with + any other operand's possible hard register use. */ + if (DECL_P (val) + && REG_P (DECL_RTL (val)) + && HARD_REGISTER_P (DECL_RTL (val))) + { + unsigned j, output_hregno = REGNO (DECL_RTL (val)); + bool early_clobber_p = strchr (constraints[i], '&') != NULL; + unsigned long match; + + /* Verify the other outputs do not use the same hard register. */ + for (j = i + 1; j < noutputs; ++j) + if (DECL_P (output_tvec[j]) + && REG_P (DECL_RTL (output_tvec[j])) + && HARD_REGISTER_P (DECL_RTL (output_tvec[j])) + && output_hregno == REGNO (DECL_RTL (output_tvec[j]))) + error ("invalid hard register usage between output operands"); + + /* Verify matching constraint operands use the same hard register + and that the non-matching constraint operands do not use the same + hard register if the output is an early clobber operand. */ + for (j = 0; j < ninputs; ++j) + if (DECL_P (input_tvec[j]) + && REG_P (DECL_RTL (input_tvec[j])) + && HARD_REGISTER_P (DECL_RTL (input_tvec[j]))) + { + unsigned input_hregno = REGNO (DECL_RTL (input_tvec[j])); + switch (*constraints[j + noutputs]) + { + case '0': case '1': case '2': case '3': case '4': + case '5': case '6': case '7': case '8': case '9': + match = strtoul (constraints[j + noutputs], NULL, 10); + break; + default: + match = ULONG_MAX; + break; + } + if (i == match + && output_hregno != input_hregno) + error ("invalid hard register usage between output operand " + "and matching constraint operand"); + else if (early_clobber_p + && i != match + && output_hregno == input_hregno) + error ("invalid hard register usage between earlyclobber " + "operand and input operand"); + } + } + if (! allows_reg && (allows_mem || is_inout Index: gcc/lra-constraints.c =================================================================== --- gcc/lra-constraints.c (revision 265402) +++ gcc/lra-constraints.c (working copy) @@ -2146,9 +2146,30 @@ process_alt_operands (int only_alternati } else { - /* Operands don't match. Both operands must - allow a reload register, otherwise we - cannot make them match. */ + /* Operands don't match. If the operands are + different user defined explicit hard registers, + then we cannot make them match. */ + if ((REG_P (*curr_id->operand_loc[nop]) + || SUBREG_P (*curr_id->operand_loc[nop])) + && (REG_P (*curr_id->operand_loc[m]) + || SUBREG_P (*curr_id->operand_loc[m]))) + { + rtx nop_reg = *curr_id->operand_loc[nop]; + if (SUBREG_P (nop_reg)) + nop_reg = SUBREG_REG (nop_reg); + rtx m_reg = *curr_id->operand_loc[m]; + if (SUBREG_P (m_reg)) + m_reg = SUBREG_REG (m_reg); + + if (HARD_REGISTER_P (nop_reg) + && REG_USERVAR_P (nop_reg) + && HARD_REGISTER_P (m_reg) + && REG_USERVAR_P (m_reg)) + break; + } + + /* Both operands must allow a reload register, + otherwise we cannot make them match. */ if (curr_alt[m] == NO_REGS) break; /* Retroactively mark the operand we had to @@ -2910,18 +2931,31 @@ process_alt_operands (int only_alternati if (first_conflict_j < 0) first_conflict_j = j; last_conflict_j = j; + /* Both the earlyclobber operand and conflicting operand + cannot both be user defined hard registers. */ + if (HARD_REGISTER_P (operand_reg[i]) + && REG_USERVAR_P (operand_reg[i]) + && operand_reg[j] != NULL_RTX + && HARD_REGISTER_P (operand_reg[j]) + && REG_USERVAR_P (operand_reg[j])) + fatal_insn ("unable to generate reloads for " + "impossible constraints:", curr_insn); } if (last_conflict_j < 0) continue; - /* If earlyclobber operand conflicts with another - non-matching operand which is actually the same register - as the earlyclobber operand, it is better to reload the - another operand as an operand matching the earlyclobber - operand can be also the same. */ - if (first_conflict_j == last_conflict_j - && operand_reg[last_conflict_j] != NULL_RTX - && ! curr_alt_match_win[last_conflict_j] - && REGNO (operand_reg[i]) == REGNO (operand_reg[last_conflict_j])) + + /* If an earlyclobber operand conflicts with another non-matching + operand (ie, they have been assigned the same hard register), + then it is better to reload the other operand, as there may + exist yet another operand with a matching constraint associated + with the earlyclobber operand. However, if one of the operands + is an explicit use of a hard register, then we must reload the + other non-hard register operand. */ + if (HARD_REGISTER_P (operand_reg[i]) + || (first_conflict_j == last_conflict_j + && operand_reg[last_conflict_j] != NULL_RTX + && !curr_alt_match_win[last_conflict_j] + && !HARD_REGISTER_P (operand_reg[last_conflict_j]))) { curr_alt_win[last_conflict_j] = false; curr_alt_dont_inherit_ops[curr_alt_dont_inherit_ops_num++]
On 10/27/18 10:24 AM, Peter Bergner wrote: > On 10/22/18 6:45 PM, Peter Bergner wrote: >> Bah, my bootstrap failed and I need to make a small change. Let me do that >> and verify my bootstraps get all the way thru before I give you the updated >> patch. Sorry. > Ok, the following updated patch survives bootstrap and regtesting on > powerpc64le-linux, x86_64-linux and s390x-linux with no regressions. > Changes from the previous patch is that checking for illegal hard register > usage in inline asm statements has been moved to expand time. Secondly, the > lra constraints code now checks for both HARD_REGISTER_P and REG_USERVAR_P. > This was because I was seeing combine forcing hard regs into a pattern > (not from inline asm) which lra needed to spill to match constraints, > which it should be able to do. > > Jeff, can you give this a try on your testers to see how it behaves on > the other arches that were having problems? > > Peter > > PR rtl-optimization/87600 > * cfgexpand.c (expand_asm_stmt): Catch illegal asm constraint usage. > * lra-constraints.c (process_alt_operands): Skip illegal hard > register usage. Prefer reloading non hard register operands. > > Index: gcc/lra-constraints.c > =================================================================== > --- gcc/lra-constraints.c (revision 265402) > +++ gcc/lra-constraints.c (working copy) > @@ -2146,9 +2146,30 @@ process_alt_operands (int only_alternati > } > else > { > - /* Operands don't match. Both operands must > - allow a reload register, otherwise we > - cannot make them match. */ > + /* Operands don't match. If the operands are > + different user defined explicit hard registers, > + then we cannot make them match. */ > + if ((REG_P (*curr_id->operand_loc[nop]) > + || SUBREG_P (*curr_id->operand_loc[nop])) > + && (REG_P (*curr_id->operand_loc[m]) > + || SUBREG_P (*curr_id->operand_loc[m]))) > + { > + rtx nop_reg = *curr_id->operand_loc[nop]; > + if (SUBREG_P (nop_reg)) > + nop_reg = SUBREG_REG (nop_reg); > + rtx m_reg = *curr_id->operand_loc[m]; > + if (SUBREG_P (m_reg)) > + m_reg = SUBREG_REG (m_reg); So the one worry I have/had in this code is nested subregs. My recollection is they do happen in rare cases. But I can also find a reference where Jim W. has indicated they're invalid (and I absolutely trust Jim on this kind of historical RTL stuff). https://gcc.gnu.org/ml/gcc/2005-04/msg01173.html Reviewing cases where we've written the stripping as a loop, several are stripping subregs as well as extractions. And I can certainly believe that we could have an RTX with some combination of subregs and extractions. The exception to that pattern would be reload which has subreg stripping loops that only strip the subregs. So, after all that, I think we're OK. It might make sense to verify we don't have nested subregs in the IL verifiers. Bonus points if you add that checking. So ideally we'd have some tests which tickle this code, even if they're target specific. So I'm OK with this patch and also OK with adding tests independently as a follow-up. jeff
On Tue, Nov 06, 2018 at 01:27:58PM -0700, Jeff Law wrote: > So the one worry I have/had in this code is nested subregs. My > recollection is they do happen in rare cases. But I can also find a > reference where Jim W. has indicated they're invalid (and I absolutely > trust Jim on this kind of historical RTL stuff). > > https://gcc.gnu.org/ml/gcc/2005-04/msg01173.html rtl.texi says @code{subreg}s of @code{subreg}s are not supported. Using @code{simplify_gen_subreg} is the recommended way to avoid this problem. (since r133982, from 2008). > So, after all that, I think we're OK. It might make sense to verify we > don't have nested subregs in the IL verifiers. Bonus points if you add > that checking. Or more general, that what is inside the subreg is a reg, because the code does rely on that. Segher
On 11/6/18 6:14 PM, Segher Boessenkool wrote: > Or more general, that what is inside the subreg is a reg, because the > code does rely on that. I think you mean to beef up the following from: + if (HARD_REGISTER_P (nop_reg) + && REG_USERVAR_P (nop_reg) + && HARD_REGISTER_P (m_reg) + && REG_USERVAR_P (m_reg)) + break; to: + if (REG_P (nop_reg) + && HARD_REGISTER_P (nop_reg) + && REG_USERVAR_P (nop_reg) + && REG_P (m_reg) + && HARD_REGISTER_P (m_reg) + && REG_USERVAR_P (m_reg)) + break; ...correct? I can add that. I don't think we need to modify the other patch hunks, since we know operand_reg[x] is already a reg. Peter
On 11/7/18 9:29 AM, Peter Bergner wrote: > On 11/6/18 6:14 PM, Segher Boessenkool wrote: >> Or more general, that what is inside the subreg is a reg, because the >> code does rely on that. > > I think you mean to beef up the following from: > > + if (HARD_REGISTER_P (nop_reg) > + && REG_USERVAR_P (nop_reg) > + && HARD_REGISTER_P (m_reg) > + && REG_USERVAR_P (m_reg)) > + break; > > to: > > + if (REG_P (nop_reg) > + && HARD_REGISTER_P (nop_reg) > + && REG_USERVAR_P (nop_reg) > + && REG_P (m_reg) > + && HARD_REGISTER_P (m_reg) > + && REG_USERVAR_P (m_reg)) > + break; > > ...correct? I can add that. I don't think we need to modify > the other patch hunks, since we know operand_reg[x] is already > a reg. I was referring to a more fundamental check in the IL checkers. Segher may have been referring to this specific code. This is obviously safe to do as well. OK with this change. jeff
On 11/7/18 11:36 AM, Jeff Law wrote: > I was referring to a more fundamental check in the IL checkers. Yes, I understood that. I was just replying to Segher's specific issue with this code. I do plan on looking at adding IL verifier checks for subregs of subregs like you requested. > Segher may have been referring to this specific code. This is obviously > safe to do as well. > > OK with this change. He was. Thanks, I'll do one more bootstrap and then commit. Thanks! Peter
On 11/7/18 11:36 AM, Jeff Law wrote:
> OK with this change.
Before I commit, how about I add the following test cases to test
both valid and invalid asm constraints? I think I have the reg
numbers for the other architectures defined correctly.
Peter
gcc/testsuite/
PR rtl-optimization/87600
* gcc.dg/pr87600.h: New.
* gcc.dg/pr87600-1.c: Likewise.
* gcc.dg/pr87600-2.c: Likewise.
Index: gcc/testsuite/gcc.dg/pr87600.h
===================================================================
--- gcc/testsuite/gcc.dg/pr87600.h (nonexistent)
+++ gcc/testsuite/gcc.dg/pr87600.h (working copy)
@@ -0,0 +1,19 @@
+#if defined (__aarch64__)
+# define REG1 "x0"
+# define REG2 "x1"
+#elif defined (__arm__)
+# define REG1 "r0"
+# define REG2 "r1"
+#elif defined (__i386__)
+# define REG1 "%eax"
+# define REG2 "%edx"
+#elif defined (__powerpc__)
+# define REG1 "r3"
+# define REG2 "r4"
+#elif defined (__s390__)
+# define REG1 "0"
+# define REG2 "1"
+#elif defined (__x86_64__)
+# define REG1 "rax"
+# define REG2 "rdx"
+#endif
Index: gcc/testsuite/gcc.dg/pr87600-1.c
===================================================================
--- gcc/testsuite/gcc.dg/pr87600-1.c (nonexistent)
+++ gcc/testsuite/gcc.dg/pr87600-1.c (working copy)
@@ -0,0 +1,52 @@
+/* PR rtl-optimization/87600 */
+/* { dg-do compile { target aarch64*-*-* arm*-*-* i?86-*-* powerpc*-*-* s390*-*-* x86_64-*-* } } */
+/* { dg-options "-O2" } */
+
+#include "pr87600.h"
+
+/* The following are all valid uses of local register variables. */
+
+long
+test0 (long arg)
+{
+ register long var asm (REG1);
+ asm ("blah %0 %1" : "+&r" (var) : "r" (arg));
+ return var;
+}
+
+long
+test1 (long arg0, long arg1)
+{
+ register long var asm (REG1);
+ asm ("blah %0, %1, %2" : "=&r" (var) : "r" (arg0), "0" (arg1));
+ return var + arg1;
+}
+
+long
+test2 (void)
+{
+ register long var1 asm (REG1);
+ register long var2 asm (REG1);
+ asm ("blah %0 %1" : "=&r" (var1) : "0" (var2));
+ return var1;
+}
+
+long
+test3 (void)
+{
+ register long var1 asm (REG1);
+ register long var2 asm (REG2);
+ long var3;
+ asm ("blah %0 %1" : "=&r" (var1), "=r" (var3) : "1" (var2));
+ return var1 + var3;
+}
+
+long
+test4 (void)
+{
+ register long var1 asm (REG1);
+ register long var2 asm (REG2);
+ register long var3 asm (REG2);
+ asm ("blah %0 %1" : "=&r" (var1), "=r" (var2) : "1" (var3));
+ return var1;
+}
Index: gcc/testsuite/gcc.dg/pr87600-2.c
===================================================================
--- gcc/testsuite/gcc.dg/pr87600-2.c (nonexistent)
+++ gcc/testsuite/gcc.dg/pr87600-2.c (working copy)
@@ -0,0 +1,44 @@
+/* PR rtl-optimization/87600 */
+/* { dg-do compile { target aarch64*-*-* arm*-*-* i?86-*-* powerpc*-*-* s390*-*-* x86_64-*-* } } */
+/* { dg-options "-O2" } */
+
+#include "pr87600.h"
+
+/* The following are all invalid uses of local register variables. */
+
+long
+test0 (void)
+{
+ register long var1 asm (REG1);
+ register long var2 asm (REG1);
+ asm ("blah %0 %1" : "=r" (var1), "=r" (var2)); /* { dg-error "invalid hard register usage between output operands" } */
+ return var1;
+}
+
+long
+test1 (void)
+{
+ register long var1 asm (REG1);
+ register long var2 asm (REG2);
+ asm ("blah %0 %1" : "=r" (var1) : "0" (var2)); /* { dg-error "invalid hard register usage between output operand and matching constraint operand" } */
+ return var1;
+}
+
+long
+test2 (void)
+{
+ register long var1 asm (REG1);
+ register long var2 asm (REG1);
+ asm ("blah %0 %1" : "=&r" (var1) : "r" (var2)); /* { dg-error "invalid hard register usage between earlyclobber operand and input operand" } */
+ return var1;
+}
+
+long
+test3 (void)
+{
+ register long var1 asm (REG1);
+ register long var2 asm (REG1);
+ long var3;
+ asm ("blah %0 %1" : "=&r" (var1), "=r" (var3) : "1" (var2)); /* { dg-error "invalid hard register usage between earlyclobber operand and input operand" } */
+ return var1 + var3;
+}
On 11/7/18 7:17 PM, Peter Bergner wrote: > On 11/7/18 11:36 AM, Jeff Law wrote: >> OK with this change. > > Before I commit, how about I add the following test cases to test > both valid and invalid asm constraints? I think I have the reg > numbers for the other architectures defined correctly. > > Peter > > > gcc/testsuite/ > PR rtl-optimization/87600 > * gcc.dg/pr87600.h: New. > * gcc.dg/pr87600-1.c: Likewise. > * gcc.dg/pr87600-2.c: Likewise. That works for me. Jeff
On 11/8/18 4:07 PM, Jeff Law wrote: > On 11/7/18 7:17 PM, Peter Bergner wrote: >> On 11/7/18 11:36 AM, Jeff Law wrote: >>> OK with this change. >> >> Before I commit, how about I add the following test cases to test >> both valid and invalid asm constraints? I think I have the reg >> numbers for the other architectures defined correctly. >> >> Peter >> >> >> gcc/testsuite/ >> PR rtl-optimization/87600 >> * gcc.dg/pr87600.h: New. >> * gcc.dg/pr87600-1.c: Likewise. >> * gcc.dg/pr87600-2.c: Likewise. > That works for me. Ok, great. The patch and test cases are now committed. Now onto the ARM problem which I have handle on. Peter
Index: gcc/lra-constraints.c =================================================================== --- gcc/lra-constraints.c (revision 264897) +++ gcc/lra-constraints.c (working copy) @@ -2904,18 +2904,29 @@ process_alt_operands (int only_alternati if (first_conflict_j < 0) first_conflict_j = j; last_conflict_j = j; + /* Both the earlyclobber operand and conflicting operand + cannot both be hard registers. */ + if (REGNO (operand_reg[i]) < FIRST_PSEUDO_REGISTER + && operand_reg[j] != NULL_RTX + && REGNO (operand_reg[j]) < FIRST_PSEUDO_REGISTER) + fatal_insn ("unable to generate reloads for:", curr_insn); } if (last_conflict_j < 0) continue; - /* If earlyclobber operand conflicts with another - non-matching operand which is actually the same register - as the earlyclobber operand, it is better to reload the - another operand as an operand matching the earlyclobber - operand can be also the same. */ - if (first_conflict_j == last_conflict_j - && operand_reg[last_conflict_j] != NULL_RTX - && ! curr_alt_match_win[last_conflict_j] - && REGNO (operand_reg[i]) == REGNO (operand_reg[last_conflict_j])) + + /* If an earlyclobber operand conflicts with another non-matching + operand (ie, they have been assigned the same hard register), + then it is better to reload the other operand, as there may + exist yet another operand with a matching constraint associated + with the earlyclobber operand. However, if one of the operands + is an explicit use of a hard register, then we must reload the + other non-hard register operand. */ + if (REGNO (operand_reg[i]) < FIRST_PSEUDO_REGISTER + || (operand_reg[last_conflict_j] != NULL_RTX + && REGNO (operand_reg[last_conflict_j]) + >= FIRST_PSEUDO_REGISTER + && first_conflict_j == last_conflict_j + && ! curr_alt_match_win[last_conflict_j])) { curr_alt_win[last_conflict_j] = false; curr_alt_dont_inherit_ops[curr_alt_dont_inherit_ops_num++]
Vlad, Jeff and Segher, I think I have determined what is happening with the aarch64 test case that is failing after my r264897 commit. It appears my patch is just exposing an issue in lra-constraints.c:process_alt_operands() when processing an insn with early clobber operands. Jeff & Segher, I have reverted all of my previous patches we were making to lra-lives.c for the analysis below and am just using straight trunk revision 264897 (ie, my last patch). The aarch64 test case is below. Note that the first 2 input/output operands are early clobber and are used with asm defined hard regs (x0 and x1). Also notice that oldval1 and oldval2 are pseudos with the same values as those in/out operands input values. long __cmpxchg_double (unsigned long arg) { unsigned long old1 = 1; unsigned long old2 = arg; unsigned long new1 = 2; unsigned long new2 = 3; void *ptr = 0; unsigned long oldval1 = old1; unsigned long oldval2 = old2; register unsigned long x0 asm ("x0") = old1; register unsigned long x1 asm ("x1") = old2; register unsigned long x2 asm ("x2") = new1; register unsigned long x3 asm ("x3") = new2; register unsigned long x4 asm ("x4") = (unsigned long) ptr; asm volatile ("casp %[old1], %[old2], %[new1], %[new2], %[v]\n" "eor %[old1], %[old1], %[oldval1]\n" "eor %[old2], %[old2], %[oldval2]\n" "orr %[old1], %[old1], %[old2]\n" : [old1] "+&r" (x0), [old2] "+&r" (x1), [v] "+Q" (* (unsigned long *) ptr) : [new1] "r" (x2), [new2] "r" (x3), [ptr] "r" (x4), [oldval1] "r" (oldval1), [oldval2] "r" (oldval2) : "x16", "x17", "x30"); return x0; } After IRA, we have the following register assignment and rtl. The only difference between before my patch and after my patch is that r92 used to be assigned to a hardreg that is not x0 - x4, which hides the lurking problem. r92 -> x1, r93 -> x5, r94 -> x6 (insn 2 3 8 2 (set (reg/v:DI 92 [arg]) (reg:DI 0 x0)) (REG_DEAD (reg:DI 0 x0))) (insn 8 2 7 2 (set (reg/v:DI 2 x2 [*x2]) (const_int 2))) (insn 7 8 6 2 (set (reg/v:DI 1 x1 [*x1]) (reg/v:DI 92 [arg]))) (insn 6 7 9 2 (set (reg/v:DI 0 x0 [*x0]) (const_int 1))) (insn 9 6 12 2 (set (reg/v:DI 3 x3 [*x3]) (const_int 3))) (insn 12 9 10 2 (set (reg:DI 94 [*x0]) (reg/v:DI 0 x0 [*x0]))) (insn 10 12 11 2 (set (reg/v:DI 4 x4 [*x4]) (const_int 0))) (insn 11 10 14 2 (set (reg/f:DI 93) (const_int 0))) (insn 14 11 21 2 (parallel [ (set (reg/v:DI 0 x0 [*x0]) (asm_operands/v:DI ("casp %0, %1, %3, %4, %2 eor %0, %0, %6 eor %1, %1, %7 orr %0, %0, %1 ") ("=&r") 0 [(reg/v:DI 2 x2 [*x2]) (reg/v:DI 3 x3 [*x3]) (reg/v:DI 4 x4 [*x4]) (reg:DI 94 [*x0]) (reg/v:DI 92 [arg]) (reg/v:DI 0 x0 [*x0]) (reg/v:DI 1 x1 [*x1]) (mem:DI (reg/f:DI 93))] [(asm_input:DI ("r")) (asm_input:DI ("r")) (asm_input:DI ("r")) (asm_input:DI ("r")) (asm_input:DI ("r")) (asm_input:DI ("0")) (asm_input:DI ("1")) (asm_input:DI ("Q"))]) (set (reg/v:DI 1 x1 [*x1]) <same mnemonics as above> ") ("=&r") 1 [<same operands as above>] (set (mem:DI (reg/f:DI 93) [1 MEM[(long unsigned intD.11 *)0B]+0 S8 A128]) <same mnemonics as above> ") ("=Q") 2 [<same operands as above>] (clobber (reg:DI 30 x30)) (clobber (reg:DI 17 x17)) (clobber (reg:DI 16 x16)) ]) "slub-min.c":17 -1 (expr_list:REG_DEAD (reg:DI 94 [*x0]) (expr_list:REG_DEAD (reg/f:DI 93) (expr_list:REG_DEAD (reg/v:DI 92 [arg]) (expr_list:REG_DEAD (reg/v:DI 4 x4 [*x4]) (expr_list:REG_DEAD (reg/v:DI 3 x3 [*x3]) (expr_list:REG_DEAD (reg/v:DI 2 x2 [*x2]) (expr_list:REG_UNUSED (reg:DI 30 x30) (expr_list:REG_UNUSED (reg:DI 17 x17) (expr_list:REG_UNUSED (reg:DI 16 x16) (expr_list:REG_UNUSED (reg/v:DI 1 x1 [*x1]) (nil)))))))))))) (insn 21 14 23 2 (use (reg/i:DI 0 x0))) In lra-constraints.c:process_alt_operands(), we notice that pseudo 92 is assigned to x1 and that an early clobber operand is also assigned to x1, or rather, that it uses x1 explicitly. This is enough to trigger reload(s), but the problem is we end up trying to reload the early clobber operand which has been forced into x1 via register asm assignment, instead of pseudo 92 which conflicts with it. The problematic code in question is: /* If earlyclobber operand conflicts with another non-matching operand which is actually the same register as the earlyclobber operand, it is better to reload the another operand as an operand matching the earlyclobber operand can be also the same. */ if (first_conflict_j == last_conflict_j && operand_reg[last_conflict_j] != NULL_RTX && ! curr_alt_match_win[last_conflict_j] && REGNO (operand_reg[i]) == REGNO (operand_reg[last_conflict_j])) { curr_alt_win[last_conflict_j] = false; curr_alt_dont_inherit_ops[curr_alt_dont_inherit_ops_num++] = last_conflict_j; losers++; /* Early clobber was already reflected in REJECT. */ lra_assert (reject > 0); if (lra_dump_file != NULL) fprintf (lra_dump_file, " %d Conflict early clobber reload: reject--\n", i); reject--; overall += LRA_LOSER_COST_FACTOR - 1; } else { /* We need to reload early clobbered register and the matched registers. */ .... In our test case, first_conflict_j == last_conflict_j and they point to the operand pseudo 92. The first three tests in the "if" condition are all true and then we check: REGNO (operand_reg[i]) == REGNO (operand_reg[last_conflict_j]) Now REGNO (operand_reg[i]) is our 2nd early clobber operand and is regno x1. However, REGNO (operand_reg[last_conflict_j]) returns regno 92, not the hard register pseudo 92 was assigned to (ie, x1). This causes us to execute the "else" clause of this code which tries to reload the early clobber operand. We end up replacing the hard coded x1 in the 2nd early clobber operand and then assign it to some reg like x7 and then the assembler barfs because it isn't x1 which it has to be. Now the REGNO (...) == REGNO (...) test predates the code that added first_conflict_j and last_conflict_j, so I think we can just eliminate that code altogether, since we've already proved there is a conflict in register usage between the early clobber operand and this non-matching operand. If I eliminate that check, we correctly reload input pseudo 92 and everything is good. Secondly, I don't think it's ever legal to reload an operand that has been hard coded by the user to a hard register via a register asm definition like in the test case above. With that in mind, what do people think of the patch below? This fixes the AARCH64 test case. However, it ICE's on the following test case: long foo (long arg) { register long var asm("x0"); asm("bla %0 %1" : "+&r"(var) : "r"(arg)); return var; } ...but that is due to a combine bug where combine replaces the use of "arg"'s pseudo in the inline asm with the incoming argument reg x0 which should be illegal. Ie, it's taking a valid inline asm and creating an invalid one since the earlyclobber op and non-matching op have both been forced to use the same hard reg. Segher has a combine patch to stop that which he is going to submit (commit?) soon. With my patch, we are also now able to catch the following user error, when before we could not: bergner@pike:~/gcc/BUGS/PR87507/AARCH64$ cat ice4.i long foo (void) { register long arg asm("x1"); register long var asm("x1"); asm ("bla %0 %1" : "=&r"(arg) : "r"(var)); return arg; } bergner@pike:~/gcc/BUGS/PR87507/AARCH64$ .../xgcc -B.../gcc -O2 -march=armv8.1-a -S ice4.i ice4.i: In function âfooâ: ice4.i:7:1: error: unable to generate reloads for: 7 | } | ^ (insn 5 6 10 2 (set (reg/v:DI 1 x1 [ arg ]) (asm_operands:DI ("bla %0 %1") ("=&r") 0 [ (reg/v:DI 1 x1 [ var ]) ] [ (asm_input:DI ("r") ice4.i:5) ] [] ice4.i:5)) "ice4.i":5 -1 (nil)) during RTL pass: reload ice4.i:7:1: internal compiler error: in process_alt_operands, at lra-constraints.c:2911 ... Thoughts? I'll note that this does not fix the S390 bugs, since those seem to be due to problems with early clobber operands and "matching" constraint operands. I'm still working on that and hope to have something soon. I am currently bootstrapping this on s390x just to make sure I don't break anything there, before debugging the existing s390x issue(s). I can't test this on aarch64 or arm, other than knowing my aarch64 cross doesn't ICE on the test case above. Peter * lra-constraints.c (process_alt_operands): Abort on illegal hard register usage. Prefer reloading non hard register operands.