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. > >
