On 4/17/19 4:19 AM, Richard Earnshaw (lists) wrote: > On 10/04/2019 23:03, Steve Ellcey wrote: >> >> Here is another patch to fix one of the failures >> listed in PR rtl-optimization/87763. This change >> fixes gcc.target/aarch64/lsl_asr_sbfiz.c by adding >> an alternative version of *ashiftsi_extv_bfiz that >> has a subreg in it. >> >> Tested with bootstrap and regression test run. >> >> OK for checkin? >> >> Steve Ellcey >> >> >> 2018-04-10 Steve Ellcey <sell...@marvell.com> >> >> PR rtl-optimization/87763 >> * config/aarch64/aarch64.md (*ashiftsi_extv_bfiz_alt): >> New Instruction. >> >> >> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md >> index e0df975..04dc06f 100644 >> --- a/gcc/config/aarch64/aarch64.md >> +++ b/gcc/config/aarch64/aarch64.md >> @@ -5634,6 +5634,22 @@ >> [(set_attr "type" "bfx")] >> ) >> >> +(define_insn "*ashiftsi_extv_bfiz_alt" >> + [(set (match_operand:SI 0 "register_operand" "=r") >> + (ashift:SI >> + (subreg:SI >> + (sign_extract:DI >> + (subreg:DI (match_operand:SI 1 "register_operand" "r") 0) >> + (match_operand 2 "aarch64_simd_shift_imm_offset_si" "n") >> + (const_int 0)) >> + 0) >> + (match_operand 3 "aarch64_simd_shift_imm_si" "n")))] >> + "IN_RANGE (INTVAL (operands[2]) + INTVAL (operands[3]), >> + 1, GET_MODE_BITSIZE (SImode) - 1)" >> + "sbfiz\\t%w0, %w1, %3, %2" >> + [(set_attr "type" "bfx")] >> +) >> + >> ;; When the bit position and width of the equivalent extraction add up to 32 >> ;; we can use a W-reg LSL instruction taking advantage of the implicit >> ;; zero-extension of the X-reg. >> > > I don't think this is right for big-endian, where the subreg offset is > not zero. Perhaps you should look at using subreg_lowpart_operator. As general guidance anytime I see a subreg in the .md files I suspect we're likely gone the wrong direction at some point.
That doesn't mean we can't use subregs, nor does it mean it's wrong in this instance, but it certainly makes me look at the changes more carefully to see if we can do something earlier or later so that we're not matching subreg expressions in the md files. I agree that in this specific case, it's likely incorrect. subreg_lowpart_* would likely help, either as a predicate or as an operator. jeff
