Double ping. Steve Ellcey sell...@marvell.com
On Tue, 2019-02-26 at 08:44 -0800, Steve Ellcey wrote: > Ping. > > Steve Ellcey > sell...@marvell.com > > > On Mon, 2019-02-11 at 10:46 -0800, Steve Ellcey wrote: > > On Thu, 2019-02-07 at 18:13 +0000, Wilco Dijkstra wrote > > > > > > Hi Steve, > > > > > > > > After special cases you could do something like t = mask2 + > > > > > (HWI_1U << shift); > > > > > return t == (t & -t) to check for a valid bfi. > > > > > > > > I am not sure I follow this logic and my attempts to use this > > > > did > > > > not > > > > work so I kept my original code. > > > > > > It's similar to the initial code in aarch64_bitmask_imm, but > > > rather > > > than adding > > > the lowest bit to the value to verify it is a mask (val + (val & > > > -val)), we use the > > > shift instead. If the shift is exactly right, it reaches the > > > first > > > set bit of the mask. > > > Adding the low bit to a valid mask always results in zero or a > > > single set bit. > > > The standard idiom to check that is t == (t & -t). > > > > > > > > + "bfi\t%<GPI:w>0, %<GPI:w>1, 0, %P2" > > > > > > > > > > This could emit a width that may be 32 too large in SImode if > > > > > bit 31 is set > > > > > (there is another use of %P in aarch64.md which may have the > > > > > same > > > > > issue). > > > > > > > > I am not sure why having bit 31 set would be a problem. Sign > > > > extension? > > > > > > Yes, if bit 31 is set, %P will emit 33 for a 32-bit constant which > > > is obviously wrong. > > > Your patch avoids this for bfi by explicitly computing the correct > > > value. > > > > > > This looks good to me (and creates useful bfi's as expected), but I > > > can't approve. > > > > > > Wilco > > > > Thanks for looking this over. I have updated the mask check to use > > your method and retested to make sure it still works. Can one of the > > aarch64 maintainers approve this patch? > > > > 2018-02-11 Steve Ellcey <sell...@marvell.com> > > > > PR rtl-optimization/87763 > > * config/aarch64/aarch64-protos.h > > (aarch64_masks_and_shift_for_bfi_p): > > New prototype. > > * config/aarch64/aarch64.c (aarch64_masks_and_shift_for_bfi_p): > > New function. > > * config/aarch64/aarch64.md (*aarch64_bfi<GPI:mode>5_shift): > > New instruction. > > (*aarch64_bfi<GPI:mode>4_noand): Ditto. > > (*aarch64_bfi<GPI:mode>4_noshift): Ditto. > > (*aarch64_bfi<GPI:mode>4_noshift_alt): Ditto. > > > > 2018-02-11 Steve Ellcey <sell...@marvell.com> > > > > PR rtl-optimization/87763 > > * gcc.target/aarch64/combine_bfxil.c: Change some bfxil checks > > to bfi. > > * gcc.target/aarch64/combine_bfi_2.c: New test. > > > >
