On Thu, 2019-04-11 at 14:58 +0000, Steve Ellcey wrote:
>
> > You've removed the ..._noshift_alt variant. That wasn't my
> > intention,
> > so perhaps you misunderstood what I was trying to say.
> >
> > The two versions are both needed, since the register tie is not
> > orthogonal to the constants used in the masks and canonicalization
> > will
> > not generate a consistent ordering of the operands.
>
> I started doing this and then convinced myself (perhaps incorrectly)
> that I didn't need the alt version. Operands 1 and 3 are registers
> and Operands 2 and 4 are constants, so the only difference is in the
> call to aarch64_masks_and_shift_for_bfi_p. Given that argument 2 to
> this call is 0 this call should be the equivelent of ((x & MASK1) |
> (y
> & MASK2)) and that should mean that:
>
> aarch64_masks_and_shift_for_bfi_p(X,0,Y) ==
> aarch64_masks_and_shift_for_bfi_p(Y,0,X)
>
>
> Maybe I am wrong about that? I will do some expirements. My testing
> did not find any cases in the testsuite where not having the _alt
> version resulted in a bfi not being generated.
OK, I think I see where my idea that I didn't need both versions is
wrong. There is an extra check on the final argument that is not done
on the initial argument. Here is the patch that I am testing, I think
it is what you have in mind. It looks wierd having arguments 3 and 4
before 1 and 2, I think that is why I had it differently in my original
patch but if you prefer this version, that is fine with me.
OK to check in once my build/test is finished?
2018-04-11 Steve Ellcey <sell...@marvell.com>
PR rtl-optimization/87763
* config/aarch64/aarch64.md (*aarch64_bfi<GPI:mode>4_noshift):
New Instruction.
diff --git a/gcc/config/aarch64/aarch64.md
b/gcc/config/aarch64/aarch64.md
index e0df975..eac688a 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -5565,7 +5565,8 @@
)
;; Like *aarch64_bfi<GPI:mode>5_shift but with no shifting, we are just
-;; copying the least significant bits of OP3 to OP0.
+;; copying the least significant bits of OP3 to OP0. We need two versions
+;; of the instruction to handle different checks on the constant values.
(define_insn "*aarch64_bfi<GPI:mode>4_noshift"
[(set (match_operand:GPI 0 "register_operand" "=r")
@@ -5579,6 +5580,18 @@
[(set_attr "type" "bfm")]
)
+(define_insn "*aarch64_bfi<GPI:mode>4_noshift"
+ [(set (match_operand:GPI 0 "register_operand" "=r")
+ (ior:GPI (and:GPI (match_operand:GPI 3 "register_operand" "0")
+ (match_operand:GPI 4 "const_int_operand" "n"))
+ (and:GPI (match_operand:GPI 1 "register_operand" "r")
+ (match_operand:GPI 2 "const_int_operand" "n"))))]
+ "aarch64_masks_and_shift_for_bfi_p (<MODE>mode, UINTVAL (operands[2]), 0,
+ UINTVAL (operands[4]))"
+ "bfi\t%<GPI:w>0, %<GPI:w>3, 0, %P4"
+ [(set_attr "type" "bfm")]
+)
+
(define_insn "*extr_insv_lower_reg<mode>"
[(set (zero_extract:GPI (match_operand:GPI 0 "register_operand" "+r")
(match_operand 1 "const_int_operand" "n")
