Hi!
On Mon, Mar 08, 2021 at 10:32:16AM -0600, acsaw...@linux.ibm.com wrote:
> PR99070 is caused by a fusion pattern matching that the individual
> instructions do not match when it is split later. In this case the
> ld+cmpi patterns were allowing a d-form load address, which the split
> condition would rightly split, however that left us with something that
> could not be matched by a ds-form ld instruction, hence the ICE. This
> only happened if the target cpu was not power10 -- if we were targeting
> power10 then a prefixed pld instruction would get generated because that
> can handle d-form. However this is not optimal code either.
>
> So the solution is a new predicate (ds_form_mem_operand) that only
> accepts what we can take as for a ds-form load. Then a small
> modification of the genfusion.pl script changes the relevant
> ld+cmpi patterns to use the new predicate.
>
> OK for trunk if bootstrap/regtest passes?
Okay. Thanks!
Some comments:
> * config/rs6000/predicates.md (ds_form_mem_operand) New
> predicate.
> * config/rs6000/genfusion.pl (gen_ld_cmpi_p10) Use
> ds_form_mem_operand in ld/lwa patterns.
You are missing the colon in thse two lines. Supposedly the serverside
scripts will not allow to push then?
> index bd6ef1e56a5..1556514263a 100644
> --- a/gcc/config/rs6000/predicates.md
> +++ b/gcc/config/rs6000/predicates.md
> @@ -992,6 +992,26 @@ (define_predicate "lwa_operand"
> return INTVAL (offset) % 4 == 0;
> })
>
> +;; Return 1 if the operand is a memory operand that has a valid address for
> +;; a DS-form instruction. I.e. the address has to be either just a register,
> +;; or register + const where the two low order bits of const are zero.
> +(define_predicate "ds_form_mem_operand"
> + (match_code "subreg,mem")
> +{
> + rtx inner, addr, offset;
> +
> + inner = op;
> + if (reload_completed && SUBREG_P (inner))
> + inner = SUBREG_REG (inner);
You just copied this from lwa_operand, but, what is that
reload_completed for? It is not clear to me, it could use a comment.
Do you know?
Should it support subregs *at all* here, since you do not allow reg
(unlike lwa_operand, which is a bit confusingly named because of that).
(The "inner" variable can just be replaced by "op" everywhere, here and
in lwa_operand, "op" is never used again).
Segher
