| Message ID | 56017AA6.20709@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
> + /* Registers that can not be allocated for this allocno, for example because > + there is an ASM_OPERANDS with that register as an output and this pseudo > + as an earlyclobber input. */ > + HARD_REG_SET forbidden_regs; You mean "with that register as an earlyclobber output and this pseudo as an input". There is no such thing as an earlyclobber input. ;-) > +asm_constraint_earlyclobber (const char *constraint) > +{ > + while (*constraint != 0) > + { > + if (*constraint == '&') > + return true; > + ++constraint; > + } > + return false; > +} This would be return strchr (constraint, '&') != NULL; but we don't actually have to search the whole string, surely. I'd be surprised if early clobber could be anywhere except at the start of the constraint. > + if (REG_P (op)) > + { > + unsigned int regno = REGNO (op); > + if (regno < FIRST_PSEUDO_REGISTER) > + for (int j = 0; j < noperands; j++) > + { > + rtx op2; > + if (j == i) > + continue; > + op2 = operands[j]; > + if (REG_P (op2) && REGNO (op2) == regno) > + return 0; > + } > + } reg_overlap_mentioned_p. r~
On 09/22/2015 12:50 PM, Richard Henderson wrote: > >> +asm_constraint_earlyclobber (const char *constraint) >> +{ >> + while (*constraint != 0) >> + { >> + if (*constraint == '&') >> + return true; >> + ++constraint; >> + } >> + return false; >> +} > > This would be > > return strchr (constraint, '&') != NULL; > > but we don't actually have to search the whole string, surely. I'd be > surprised if early clobber could be anywhere except at the start of the constraint. And doesn't it have to be =& and always at the start for an asm constraint? For operands in patterns, we have to consider operand alternatives, but that's not on issue here.
On 09/22/2015 12:12 PM, Jeff Law wrote: > On 09/22/2015 12:50 PM, Richard Henderson wrote: >> >>> +asm_constraint_earlyclobber (const char *constraint) >>> +{ >>> + while (*constraint != 0) >>> + { >>> + if (*constraint == '&') >>> + return true; >>> + ++constraint; >>> + } >>> + return false; >>> +} >> >> This would be >> >> return strchr (constraint, '&') != NULL; >> >> but we don't actually have to search the whole string, surely. I'd be >> surprised if early clobber could be anywhere except at the start of the >> constraint. > And doesn't it have to be =& and always at the start for an asm constraint? > For operands in patterns, we have to consider operand alternatives, but that's > not on issue here. Technically we do support alternatives on asms. Not usefully except for CISC, since all alternatives have to use the same output pattern, but one can write things like asm("add %0,%1" : "=r,m"(x) : "rim,ri"(y)) r~
On 09/22/2015 01:15 PM, Richard Henderson wrote: >> And doesn't it have to be =& and always at the start for an asm constraint? >> For operands in patterns, we have to consider operand alternatives, but that's >> not on issue here. > > Technically we do support alternatives on asms. Not usefully except for CISC, > since all alternatives have to use the same output pattern, but one can write > things like > > asm("add %0,%1" : "=r,m"(x) : "rim,ri"(y)) Wow, learn something every day. In that case, I think we do have to search a little harder than just peeking at the first two letters of the constraint :( jeff
On 09/22/2015 12:18 PM, Jeff Law wrote: > On 09/22/2015 01:15 PM, Richard Henderson wrote: >>> And doesn't it have to be =& and always at the start for an asm constraint? >>> For operands in patterns, we have to consider operand alternatives, but that's >>> not on issue here. >> >> Technically we do support alternatives on asms. Not usefully except for CISC, >> since all alternatives have to use the same output pattern, but one can write >> things like >> >> asm("add %0,%1" : "=r,m"(x) : "rim,ri"(y)) > Wow, learn something every day. In that case, I think we do have to search a > little harder than just peeking at the first two letters of the constraint :( We wouldn't be. He's adding this check at the beginning of each alternative. So why would we want to search the entire rest of the string now? r~
On 09/22/2015 01:20 PM, Richard Henderson wrote: > On 09/22/2015 12:18 PM, Jeff Law wrote: >> On 09/22/2015 01:15 PM, Richard Henderson wrote: >>>> And doesn't it have to be =& and always at the start for an asm constraint? >>>> For operands in patterns, we have to consider operand alternatives, but that's >>>> not on issue here. >>> >>> Technically we do support alternatives on asms. Not usefully except for CISC, >>> since all alternatives have to use the same output pattern, but one can write >>> things like >>> >>> asm("add %0,%1" : "=r,m"(x) : "rim,ri"(y)) >> Wow, learn something every day. In that case, I think we do have to search a >> little harder than just peeking at the first two letters of the constraint :( > > We wouldn't be. He's adding this check at the beginning of each alternative. > So why would we want to search the entire rest of the string now? If he's walking down each alternative (I didn't look), then just checking the start of each alternative would obviously be OK. I made the assumption he was searching the entire constraint string. Jeff
Index: gcc/testsuite/gcc.target/i386/explicit-register-earlyclobber.c =================================================================== --- gcc/testsuite/gcc.target/i386/explicit-register-earlyclobber.c (revision 0) +++ gcc/testsuite/gcc.target/i386/explicit-register-earlyclobber.c (revision 0) @@ -0,0 +1,18 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ +/* { dg-final { scan-assembler-times "operand0:%eax" 2 } } */ + +static int test(int num) +{ + register int v0 __asm__("eax"); + // We want to put operand0 in eax and put operand1 elsewhere. + asm volatile ("# Result: operand0:%0 operand1:%1" :"=&r"(v0): "r"(num)); + return v0; +} + +int t2() +{ + int i = test(1234); + int j = test(i); + return j; +} Index: gcc/ira-int.h =================================================================== --- gcc/ira-int.h (revision 228019) +++ gcc/ira-int.h (working copy) @@ -380,6 +380,10 @@ struct ira_allocno int cheap_calls_crossed_num; /* Registers clobbered by intersected calls. */ HARD_REG_SET crossed_calls_clobbered_regs; + /* Registers that can not be allocated for this allocno, for example because + there is an ASM_OPERANDS with that register as an output and this pseudo + as an earlyclobber input. */ + HARD_REG_SET forbidden_regs; /* Array of usage costs (accumulated and the one updated during coloring) for each hard register of the allocno class. The member value can be NULL if all costs are the same and equal to @@ -440,6 +444,7 @@ struct ira_allocno #define ALLOCNO_WMODE(A) ((A)->wmode) #define ALLOCNO_PREFS(A) ((A)->allocno_prefs) #define ALLOCNO_COPIES(A) ((A)->allocno_copies) +#define ALLOCNO_FORBIDDEN_REGS(A) ((A)->forbidden_regs) #define ALLOCNO_HARD_REG_COSTS(A) ((A)->hard_reg_costs) #define ALLOCNO_UPDATED_HARD_REG_COSTS(A) ((A)->updated_hard_reg_costs) #define ALLOCNO_CONFLICT_HARD_REG_COSTS(A) \ Index: gcc/ira-color.c =================================================================== --- gcc/ira-color.c (revision 228019) +++ gcc/ira-color.c (working copy) @@ -1584,6 +1584,9 @@ check_hard_reg_p (ira_allocno_t a, int h /* Checking only profitable hard regs. */ if (! TEST_HARD_REG_BIT (profitable_regs, hard_regno)) return false; + /* Don't use a forbidden hard reg. */ + if (TEST_HARD_REG_BIT (ALLOCNO_FORBIDDEN_REGS (a), hard_regno)) + return false; nregs = hard_regno_nregs[hard_regno][mode]; nwords = ALLOCNO_NUM_OBJECTS (a); for (j = 0; j < nregs; j++) Index: gcc/recog.c =================================================================== --- gcc/recog.c (revision 228019) +++ gcc/recog.c (working copy) @@ -139,6 +139,19 @@ asm_labels_ok (rtx body) return true; } +bool +asm_constraint_earlyclobber (const char *constraint) +{ + while (*constraint != 0) + { + if (*constraint == '&') + return true; + ++constraint; + } + return false; +} + + /* Check that X is an insn-body for an `asm' with operands and that the operands mentioned in it are legitimate. */ @@ -179,6 +192,24 @@ check_asm_operands (rtx x) const char *c = constraints[i]; if (c[0] == '%') c++; + if (asm_constraint_earlyclobber (c)) + { + rtx op = operands[i]; + if (REG_P (op)) + { + unsigned int regno = REGNO (op); + if (regno < FIRST_PSEUDO_REGISTER) + for (int j = 0; j < noperands; j++) + { + rtx op2; + if (j == i) + continue; + op2 = operands[j]; + if (REG_P (op2) && REGNO (op2) == regno) + return 0; + } + } + } if (! asm_operand_ok (operands[i], c, constraints)) return 0; } Index: gcc/recog.h =================================================================== --- gcc/recog.h (revision 228019) +++ gcc/recog.h (working copy) @@ -84,6 +84,7 @@ alternative_class (const operand_alterna extern void init_recog (void); extern void init_recog_no_volatile (void); +extern bool asm_constraint_earlyclobber (const char *); extern int check_asm_operands (rtx); extern int asm_operand_ok (rtx, const char *, const char **); extern bool validate_change (rtx, rtx *, rtx, bool); Index: gcc/ira-build.c =================================================================== --- gcc/ira-build.c (revision 228019) +++ gcc/ira-build.c (working copy) @@ -512,6 +512,7 @@ ira_create_allocno (int regno, bool cap_ ALLOCNO_CALLS_CROSSED_NUM (a) = 0; ALLOCNO_CHEAP_CALLS_CROSSED_NUM (a) = 0; CLEAR_HARD_REG_SET (ALLOCNO_CROSSED_CALLS_CLOBBERED_REGS (a)); + CLEAR_HARD_REG_SET (ALLOCNO_FORBIDDEN_REGS (a)); #ifdef STACK_REGS ALLOCNO_NO_STACK_REG_P (a) = false; ALLOCNO_TOTAL_NO_STACK_REG_P (a) = false; Index: gcc/ira-costs.c =================================================================== --- gcc/ira-costs.c (revision 228019) +++ gcc/ira-costs.c (working copy) @@ -1983,6 +1983,62 @@ find_costs_and_classes (FILE *dump_file) +/* Subroutine of process_bb_node_for_asm_operands. Processes a single + asm_operands set. */ + +static void +process_asm_operands_set (rtx set) +{ + if (GET_CODE (set) != SET) + return; + rtx dst = SET_DEST (set); + if (! REG_P (dst)) + return; + int dst_regno = REGNO (dst); + if (dst_regno >= FIRST_PSEUDO_REGISTER) + return; + + int noperands = asm_noperands (set); + if (noperands < 1) + return; + rtx asmop = SET_SRC (set); + if (!asm_constraint_earlyclobber (ASM_OPERANDS_OUTPUT_CONSTRAINT (asmop))) + return; + for (int i = 0; i < ASM_OPERANDS_INPUT_LENGTH (asmop); i++) + { + rtx src = ASM_OPERANDS_INPUT (asmop, i); + if (! REG_P (src)) + continue; + int src_regno = REGNO (src); + if (src_regno < FIRST_PSEUDO_REGISTER) + continue; + ira_allocno_t a = ira_curr_regno_allocno_map[src_regno]; + SET_HARD_REG_BIT (ALLOCNO_FORBIDDEN_REGS (a), dst_regno); + } +} + +/* Process insns with asm_operands patterns to forbid allocations that would + violate earlyclobber rules. */ +static void +process_bb_node_for_asm_operands (ira_loop_tree_node_t loop_tree_node) +{ + basic_block bb = loop_tree_node->bb; + if (bb == NULL) + return; + rtx_insn *insn; + FOR_BB_INSNS (bb, insn) + { + if (!NONDEBUG_INSN_P (insn) || !INSN_P (insn)) + continue; + rtx body = PATTERN (insn); + if (GET_CODE (body) == PARALLEL) + for (int i = 0; i < XVECLEN (body, 0); i++) + process_asm_operands_set (XVECEXP (body, 0, i)); + else + process_asm_operands_set (body); + } +} + /* Process moves involving hard regs to modify allocno hard register costs. We can do this only after determining allocno class. If a hard register forms a register class, then moves with the hard @@ -2118,6 +2174,8 @@ setup_allocno_class_and_costs (void) } } } + ira_traverse_loop_tree (true, ira_loop_tree_root, + process_bb_node_for_asm_operands, NULL); if (optimize) ira_traverse_loop_tree (true, ira_loop_tree_root, process_bb_node_for_hard_reg_moves, NULL);
When using an asm register variable as an input or output operand for an inline assembler block, GCC guarantees that the specified register is used for the operand in question (see https://gcc.gnu.org/onlinedocs/gcc/Local-Reg-Vars.html). Together with earlyclobber constraints (https://gcc.gnu.org/onlinedocs/gcc/Modifiers.html) this creates the possibility of the register allocator being disallowed from using certain registers for asm_operands insns. IRA does not currently support this scenario and generates code that breaks the explicit register asm guarantee (putting operand1 in %eax in the attached testcase). I propose to fix this by adding a HARD_REG_SET member to struct ira_allocno listing the hard registers that are forbidden from being used for the allocation in question. This is preferable to just increasing the cost to a very high number, because if the user writes inline assembly that it is impossible to generate correct code for, then it's better for GCC to fail rather than silently generate incorrect code. In addition to the register allocator, the combine pass was also violating this rule. This patch also changes check_asm_operands() in recog.c to recognize this before reload and disallow it. Bootstrapped on x86_64 with no regressions. The testcase is necessarily target-dependent, so I've added one for i386 but I have also manually tested that the fix works and has the desired effect on MIPS. Ok to commit? Thanks, Andrew 2015-09-22 Andrew Jenner <andrew@codesourcery.com> gcc/ * ira-int.h (struct ira_allocno): Add forbidden_regs. (ALLOCNO_FORBIDDEN_REGS): Define. * ira-color.c (check_hard_reg_p): Use it. * ira-build.c (ira_create_allocno): Clear it. * ira-costs.c (process_asm_operands_sew): New function. (process_bb_node_for_asm_operands): New function. (setup_allocno_class_and_costs): Use it. * recog.c (asm_constraint_earlyclobber): New function. (check_asm_operands): Call it. * recog.h (asm_constraint_earlyclobber): Declare. gcc/testsuite/ * gcc.target/i386/explicit-register-earlyclobber.c: New file.