Message ID | 1431350621-21405-3-git-send-email-krebbel@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
On 05/11/2015 07:23 AM, Andreas Krebbel wrote: > The current implementation re-uses the location of the selection > pattern to generate a new one. This fails if the pattern resides in a > read-only location. With the patch a new temporary register is > allocated for that purpose. > > gcc/ > * optabs.c (expand_vec_perm): Allocate a temp reg for the new > select pattern. This is probably a good idea even if SEL is not in readonly memory. When possible we still want rtl objects to have a single def. Presumably it passes a bootstrap and regression test. OK for the trunk. As for the branches, that's the call of the release managers. jeff
On 05/11/2015 06:23 AM, Andreas Krebbel wrote: > @@ -6784,14 +6784,18 @@ expand_vec_perm (machine_mode mode, rtx v0, rtx v1, rtx sel, rtx target) > { > /* Multiply each element by its byte size. */ > machine_mode selmode = GET_MODE (sel); > + /* We cannot re-use SEL as a temp operand since it might by in > + read-only storage. */ > + rtx sel_reg = gen_reg_rtx (selmode); > + > if (u == 2) > - sel = expand_simple_binop (selmode, PLUS, sel, sel, > - sel, 0, OPTAB_DIRECT); > + sel_reg = expand_simple_binop (selmode, PLUS, sel, sel, > + sel_reg, 0, OPTAB_DIRECT); > else You needn't allocate sel_reg explicitly; expand_simple_binop will do that for you if the TARGET parameter is NULL. Thus this patch should be an 8 character change on those two calls. r~
On 05/18/2015 07:35 PM, Richard Henderson wrote: > On 05/11/2015 06:23 AM, Andreas Krebbel wrote: >> @@ -6784,14 +6784,18 @@ expand_vec_perm (machine_mode mode, rtx v0, rtx v1, rtx sel, rtx target) >> { >> /* Multiply each element by its byte size. */ >> machine_mode selmode = GET_MODE (sel); >> + /* We cannot re-use SEL as a temp operand since it might by in >> + read-only storage. */ >> + rtx sel_reg = gen_reg_rtx (selmode); >> + >> if (u == 2) >> - sel = expand_simple_binop (selmode, PLUS, sel, sel, >> - sel, 0, OPTAB_DIRECT); >> + sel_reg = expand_simple_binop (selmode, PLUS, sel, sel, >> + sel_reg, 0, OPTAB_DIRECT); >> else > > You needn't allocate sel_reg explicitly; expand_simple_binop will do that for > you if the TARGET parameter is NULL. > > Thus this patch should be an 8 character change on those two calls. Right. Thanks! Ok to apply with that change? -Andreas-
On 05/19/2015 01:41 AM, Andreas Krebbel wrote: > On 05/18/2015 07:35 PM, Richard Henderson wrote: >> On 05/11/2015 06:23 AM, Andreas Krebbel wrote: >>> @@ -6784,14 +6784,18 @@ expand_vec_perm (machine_mode mode, rtx v0, rtx v1, rtx sel, rtx target) >>> { >>> /* Multiply each element by its byte size. */ >>> machine_mode selmode = GET_MODE (sel); >>> + /* We cannot re-use SEL as a temp operand since it might by in >>> + read-only storage. */ >>> + rtx sel_reg = gen_reg_rtx (selmode); >>> + >>> if (u == 2) >>> - sel = expand_simple_binop (selmode, PLUS, sel, sel, >>> - sel, 0, OPTAB_DIRECT); >>> + sel_reg = expand_simple_binop (selmode, PLUS, sel, sel, >>> + sel_reg, 0, OPTAB_DIRECT); >>> else >> >> You needn't allocate sel_reg explicitly; expand_simple_binop will do that for >> you if the TARGET parameter is NULL. >> >> Thus this patch should be an 8 character change on those two calls. > > Right. Thanks! > > Ok to apply with that change? Yes, thanks. r~
diff --git a/gcc/optabs.c b/gcc/optabs.c index 983c8d9..8926efa 100644 --- a/gcc/optabs.c +++ b/gcc/optabs.c @@ -6784,14 +6784,18 @@ expand_vec_perm (machine_mode mode, rtx v0, rtx v1, rtx sel, rtx target) { /* Multiply each element by its byte size. */ machine_mode selmode = GET_MODE (sel); + /* We cannot re-use SEL as a temp operand since it might by in + read-only storage. */ + rtx sel_reg = gen_reg_rtx (selmode); + if (u == 2) - sel = expand_simple_binop (selmode, PLUS, sel, sel, - sel, 0, OPTAB_DIRECT); + sel_reg = expand_simple_binop (selmode, PLUS, sel, sel, + sel_reg, 0, OPTAB_DIRECT); else - sel = expand_simple_binop (selmode, ASHIFT, sel, - GEN_INT (exact_log2 (u)), - sel, 0, OPTAB_DIRECT); - gcc_assert (sel != NULL); + sel_reg = expand_simple_binop (selmode, ASHIFT, sel, + GEN_INT (exact_log2 (u)), + sel_reg, 0, OPTAB_DIRECT); + gcc_assert (sel_reg != NULL); /* Broadcast the low byte each element into each of its bytes. */ vec = rtvec_alloc (w); @@ -6803,7 +6807,7 @@ expand_vec_perm (machine_mode mode, rtx v0, rtx v1, rtx sel, rtx target) RTVEC_ELT (vec, i) = GEN_INT (this_e); } tmp = gen_rtx_CONST_VECTOR (qimode, vec); - sel = gen_lowpart (qimode, sel); + sel = gen_lowpart (qimode, sel_reg); sel = expand_vec_perm (qimode, sel, sel, tmp, NULL); gcc_assert (sel != NULL);