Hi!
On Fri, May 13, 2022 at 10:40:22AM +0800, Kewen.Lin wrote:
> on 2022/5/13 09:07, HAO CHEN GUI wrote:
> > * config/rs6000/rs6000.md (extenddi_ca_minus_one): Define.
>
> Nit: (*extenddi_ca_minus_one): New define_insn_and_split.
Or just "New." even :-) It's boring, yes, but boring is good. It helps
to be more verbose sometimes, certainly, but it isn't required here.
Changelog entries are free-form in any case, after the colons that is :-)
> > +/* { dg-options "-O2 -mno-isel" } */
>
> Nit: It seems good to put one comment for the reason why we need this
> special -mno-isel, like something: Power9 leverages isel for this case,
> force -mno-isel to keep the test point valid on Power9 and later."
But in fewer words if you can :-) Even "/* for p9 and later */" would
be very helpful already. It's not necessary to explain everything, but
giving some leads to whoever gets to investigate this testcase next is
useful :-)
Segher
