* [PATCH, AArch64] Support BFI instruction and insv standard pattern
@ 2013-05-08 9:58 Ian Bolton
2013-05-20 18:56 ` Ian Bolton
0 siblings, 1 reply; 5+ messages in thread
From: Ian Bolton @ 2013-05-08 9:58 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 476 bytes --]
Hi,
This patch implements the BFI variant of BFM. In doing so, it also
implements the insv standard pattern.
I've regression tested on bare-metal and linux.
It comes complete with its own compilation and execution testcase.
OK for trunk?
Cheers,
Ian
2013-05-08 Ian Bolton <ian.bolton@arm.com>
gcc/
* config/aarch64/aarch64.md (insv): New define_expand.
(*insv_reg<mode>): New define_insn.
testsuite/
* gcc.target/aarch64/bfm_1.c: New test.
[-- Attachment #2: aarch64-insv-patch-v6.txt --]
[-- Type: text/plain, Size: 3156 bytes --]
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 330f78c..b730ed0 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -3118,6 +3118,53 @@
(set_attr "mode" "<MODE>")]
)
+;; Bitfield Insert (insv)
+(define_expand "insv<mode>"
+ [(set (zero_extract:GPI (match_operand:GPI 0 "register_operand")
+ (match_operand 1 "const_int_operand")
+ (match_operand 2 "const_int_operand"))
+ (match_operand:GPI 3 "general_operand"))]
+ ""
+{
+ HOST_WIDE_INT mask = ((HOST_WIDE_INT)1 << INTVAL (operands[1])) - 1;
+
+ if (GET_MODE_BITSIZE (<MODE>mode) > BITS_PER_WORD
+ || INTVAL (operands[1]) < 1
+ || INTVAL (operands[1]) >= GET_MODE_BITSIZE (<MODE>mode)
+ || INTVAL (operands[2]) < 0
+ || (INTVAL (operands[2]) + INTVAL (operands[1]))
+ > GET_MODE_BITSIZE (<MODE>mode))
+ FAIL;
+
+ /* Prefer AND/OR for inserting all zeros or all ones. */
+ if (CONST_INT_P (operands[3])
+ && ((INTVAL (operands[3]) & mask) == 0
+ || (INTVAL (operands[3]) & mask) == mask))
+ FAIL;
+
+ if (!register_operand (operands[3], <MODE>mode))
+ operands[3] = force_reg (<MODE>mode, operands[3]);
+
+ /* Intentional fall-through, which will lead to below pattern
+ being matched. */
+})
+
+(define_insn "*insv_reg<mode>"
+ [(set (zero_extract:GPI (match_operand:GPI 0 "register_operand" "+r")
+ (match_operand 1 "const_int_operand" "n")
+ (match_operand 2 "const_int_operand" "n"))
+ (match_operand:GPI 3 "register_operand" "r"))]
+ "!(GET_MODE_BITSIZE (<MODE>mode) > BITS_PER_WORD
+ || INTVAL (operands[1]) < 1
+ || INTVAL (operands[1]) >= GET_MODE_BITSIZE (<MODE>mode)
+ || INTVAL (operands[2]) < 0
+ || (INTVAL (operands[2]) + INTVAL (operands[1]))
+ > GET_MODE_BITSIZE (<MODE>mode))"
+ "bfi\\t%<w>0, %<w>3, %2, %1"
+ [(set_attr "v8type" "bfm")
+ (set_attr "mode" "<MODE>")]
+)
+
(define_insn "*<optab><ALLX:mode>_shft_<GPI:mode>"
[(set (match_operand:GPI 0 "register_operand" "=r")
(ashift:GPI (ANY_EXTEND:GPI
diff --git a/gcc/testsuite/gcc.target/aarch64/bfm_1.c b/gcc/testsuite/gcc.target/aarch64/bfm_1.c
new file mode 100644
index 0000000..d9a73a1
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/bfm_1.c
@@ -0,0 +1,46 @@
+/* { dg-do run } */
+/* { dg-options "-O2 --save-temps -fno-inline" } */
+
+extern void abort (void);
+
+typedef struct bitfield
+{
+ unsigned short eight: 8;
+ unsigned short four: 4;
+ unsigned short five: 5;
+ unsigned short seven: 7;
+} bitfield;
+
+bitfield
+bfi1 (bitfield a)
+{
+ /* { dg-final { scan-assembler "bfi\tw\[0-9\]+, w\[0-9\]+, 0, 8" } } */
+ a.eight = 3;
+ return a;
+}
+
+bitfield
+bfi2 (bitfield a)
+{
+ /* { dg-final { scan-assembler "bfi\tw\[0-9\]+, w\[0-9\]+, 16, 5" } } */
+ a.five = 7;
+ return a;
+}
+
+int
+main (int argc, char** argv)
+{
+ bitfield a;
+ bitfield b = bfi1 (a);
+ bitfield c = bfi2 (b);
+
+ if (c.eight != 3)
+ abort ();
+
+ if (c.five != 7)
+ abort ();
+
+ return 0;
+}
+
+/* { dg-final { cleanup-saved-temps } } */
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH, AArch64] Support BFI instruction and insv standard pattern
2013-05-08 9:58 [PATCH, AArch64] Support BFI instruction and insv standard pattern Ian Bolton
@ 2013-05-20 18:56 ` Ian Bolton
2013-05-20 19:36 ` Richard Henderson
0 siblings, 1 reply; 5+ messages in thread
From: Ian Bolton @ 2013-05-20 18:56 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1030 bytes --]
> Hi,
>
> This patch implements the BFI variant of BFM. In doing so, it also
> implements the insv standard pattern.
>
> I've regression tested on bare-metal and linux.
>
> It comes complete with its own compilation and execution testcase.
>
> OK for trunk?
>
> Cheers,
> Ian
>
>
> 2013-05-08 Ian Bolton <ian.bolton@arm.com>
>
> gcc/
> * config/aarch64/aarch64.md (insv): New define_expand.
> (*insv_reg<mode>): New define_insn.
>
> testsuite/
> * gcc.target/aarch64/bfm_1.c: New test.
(This patch did not yet get commit approval.)
I improved this patch during the work I did on the recent insv_imm patch
(http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01007.html).
I also renamed the testcase.
Regression testing completed successfully.
OK for trunk?
Cheers,
Ian
2013-05-20 Ian Bolton <ian.bolton@arm.com>
gcc/
* config/aarch64/aarch64.md (insv): New define_expand.
(*insv_reg<mode>): New define_insn.
testsuite/
* gcc.target/aarch64/insv_1.c: New test.
[-- Attachment #2: aarch64-insv-patch-v11.txt --]
[-- Type: text/plain, Size: 3760 bytes --]
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index b27bcda..e5d6950 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -3164,6 +3164,52 @@
(set_attr "mode" "<MODE>")]
)
+;; Bitfield Insert (insv)
+(define_expand "insv<mode>"
+ [(set (zero_extract:GPI (match_operand:GPI 0 "register_operand")
+ (match_operand 1 "const_int_operand")
+ (match_operand 2 "const_int_operand"))
+ (match_operand:GPI 3 "general_operand"))]
+ ""
+{
+ unsigned HOST_WIDE_INT width = UINTVAL (operands[1]);
+ unsigned HOST_WIDE_INT pos = UINTVAL (operands[2]);
+ rtx value = operands[3];
+
+ if (width == 0 || (pos + width) > GET_MODE_BITSIZE (<MODE>mode))
+ FAIL;
+
+ if (CONST_INT_P (value))
+ {
+ unsigned HOST_WIDE_INT mask = ((unsigned HOST_WIDE_INT)1 << width) - 1;
+
+ /* Prefer AND/OR for inserting all zeros or all ones. */
+ if ((UINTVAL (value) & mask) == 0
+ || (UINTVAL (value) & mask) == mask)
+ FAIL;
+
+ /* Force the constant into a register, unless this is a 16-bit aligned
+ 16-bit wide insert, which is handled by insv_imm. */
+ if (width != 16 || (pos % 16) != 0)
+ operands[3] = force_reg (<MODE>mode, value);
+ }
+ else if (!register_operand (value, <MODE>mode))
+ operands[3] = force_reg (<MODE>mode, value);
+})
+
+(define_insn "*insv_reg<mode>"
+ [(set (zero_extract:GPI (match_operand:GPI 0 "register_operand" "+r")
+ (match_operand 1 "const_int_operand" "n")
+ (match_operand 2 "const_int_operand" "n"))
+ (match_operand:GPI 3 "register_operand" "r"))]
+ "!(UINTVAL (operands[1]) == 0
+ || (UINTVAL (operands[2]) + UINTVAL (operands[1])
+ > GET_MODE_BITSIZE (<MODE>mode)))"
+ "bfi\\t%<w>0, %<w>3, %2, %1"
+ [(set_attr "v8type" "bfm")
+ (set_attr "mode" "<MODE>")]
+)
+
(define_insn "*<optab><ALLX:mode>_shft_<GPI:mode>"
[(set (match_operand:GPI 0 "register_operand" "=r")
(ashift:GPI (ANY_EXTEND:GPI
diff --git a/gcc/testsuite/gcc.target/aarch64/insv_1.c b/gcc/testsuite/gcc.target/aarch64/insv_1.c
new file mode 100644
index 0000000..0977e15
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/insv_1.c
@@ -0,0 +1,84 @@
+/* { dg-do run } */
+/* { dg-options "-O2 --save-temps -fno-inline" } */
+
+extern void abort (void);
+
+typedef struct bitfield
+{
+ unsigned short eight: 8;
+ unsigned short four: 4;
+ unsigned short five: 5;
+ unsigned short seven: 7;
+ unsigned int sixteen: 16;
+} bitfield;
+
+bitfield
+bfi1 (bitfield a)
+{
+ /* { dg-final { scan-assembler "bfi\tx\[0-9\]+, x\[0-9\]+, 0, 8" } } */
+ a.eight = 3;
+ return a;
+}
+
+bitfield
+bfi2 (bitfield a)
+{
+ /* { dg-final { scan-assembler "bfi\tx\[0-9\]+, x\[0-9\]+, 16, 5" } } */
+ a.five = 7;
+ return a;
+}
+
+bitfield
+movk (bitfield a)
+{
+ /* { dg-final { scan-assembler "movk\tx\[0-9\]+, 0x1d6b, lsl 32" } } */
+ a.sixteen = 7531;
+ return a;
+}
+
+bitfield
+set1 (bitfield a)
+{
+ /* { dg-final { scan-assembler "orr\tx\[0-9\]+, x\[0-9\]+, 2031616" } } */
+ a.five = 0x1f;
+ return a;
+}
+
+bitfield
+set0 (bitfield a)
+{
+ /* { dg-final { scan-assembler "and\tx\[0-9\]+, x\[0-9\]+, -2031617" } } */
+ a.five = 0;
+ return a;
+}
+
+
+int
+main (int argc, char** argv)
+{
+ static bitfield a;
+ bitfield b = bfi1 (a);
+ bitfield c = bfi2 (b);
+ bitfield d = movk (c);
+
+ if (d.eight != 3)
+ abort ();
+
+ if (d.five != 7)
+ abort ();
+
+ if (d.sixteen != 7531)
+ abort ();
+
+ d = set1 (d);
+ if (d.five != 0x1f)
+ abort ();
+
+ d = set0 (d);
+ if (d.five != 0)
+ abort ();
+
+ return 0;
+}
+
+/* { dg-final { cleanup-saved-temps } } */
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH, AArch64] Support BFI instruction and insv standard pattern
2013-05-20 18:56 ` Ian Bolton
@ 2013-05-20 19:36 ` Richard Henderson
2013-05-30 11:33 ` Ian Bolton
0 siblings, 1 reply; 5+ messages in thread
From: Richard Henderson @ 2013-05-20 19:36 UTC (permalink / raw)
To: Ian Bolton; +Cc: gcc-patches
On 05/20/2013 11:55 AM, Ian Bolton wrote:
> I improved this patch during the work I did on the recent insv_imm patch
> (http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01007.html).
Thanks, you cleaned up almost everything on which I would have commented
with the previous patch revision. The only thing left is:
> + else if (!register_operand (value, <MODE>mode))
> + operands[3] = force_reg (<MODE>mode, value);
Checking register_operand before force_reg is unnecessary; you're not saving a
function call, and force_reg will itself perform the register check.
r~
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH, AArch64] Support BFI instruction and insv standard pattern
2013-05-20 19:36 ` Richard Henderson
@ 2013-05-30 11:33 ` Ian Bolton
2013-05-30 14:19 ` Richard Henderson
0 siblings, 1 reply; 5+ messages in thread
From: Ian Bolton @ 2013-05-30 11:33 UTC (permalink / raw)
To: 'Richard Henderson', gcc-patches
[-- Attachment #1: Type: text/plain, Size: 988 bytes --]
> On 05/20/2013 11:55 AM, Ian Bolton wrote:
> > I improved this patch during the work I did on the recent insv_imm
> patch
> > (http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01007.html).
>
> Thanks, you cleaned up almost everything on which I would have
> commented
> with the previous patch revision. The only thing left is:
>
> > + else if (!register_operand (value, <MODE>mode))
> > + operands[3] = force_reg (<MODE>mode, value);
>
> Checking register_operand before force_reg is unnecessary; you're not
> saving a
> function call, and force_reg will itself perform the register check.
Thanks for the review, Richard.
Latest patch is attached, which fixes this.
Linux and bare-metal regression runs successful.
OK for trunk?
Cheers,
Ian
2013-05-30 Ian Bolton <ian.bolton@arm.com>
gcc/
* config/aarch64/aarch64.md (insv): New define_expand.
(*insv_reg<mode>): New define_insn.
testsuite/
* gcc.target/aarch64/insv_1.c: New test.
[-- Attachment #2: aarch64-insv-patch-130530.txt --]
[-- Type: text/plain, Size: 3603 bytes --]
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 2bdbfa9..89db092 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -3163,6 +3163,50 @@
(set_attr "mode" "<MODE>")]
)
+;; Bitfield Insert (insv)
+(define_expand "insv<mode>"
+ [(set (zero_extract:GPI (match_operand:GPI 0 "register_operand")
+ (match_operand 1 "const_int_operand")
+ (match_operand 2 "const_int_operand"))
+ (match_operand:GPI 3 "general_operand"))]
+ ""
+{
+ unsigned HOST_WIDE_INT width = UINTVAL (operands[1]);
+ unsigned HOST_WIDE_INT pos = UINTVAL (operands[2]);
+ rtx value = operands[3];
+
+ if (width == 0 || (pos + width) > GET_MODE_BITSIZE (<MODE>mode))
+ FAIL;
+
+ if (CONST_INT_P (value))
+ {
+ unsigned HOST_WIDE_INT mask = ((unsigned HOST_WIDE_INT)1 << width) - 1;
+
+ /* Prefer AND/OR for inserting all zeros or all ones. */
+ if ((UINTVAL (value) & mask) == 0
+ || (UINTVAL (value) & mask) == mask)
+ FAIL;
+
+ /* 16-bit aligned 16-bit wide insert is handled by insv_imm. */
+ if (width == 16 && (pos % 16) == 0)
+ DONE;
+ }
+ operands[3] = force_reg (<MODE>mode, value);
+})
+
+(define_insn "*insv_reg<mode>"
+ [(set (zero_extract:GPI (match_operand:GPI 0 "register_operand" "+r")
+ (match_operand 1 "const_int_operand" "n")
+ (match_operand 2 "const_int_operand" "n"))
+ (match_operand:GPI 3 "register_operand" "r"))]
+ "!(UINTVAL (operands[1]) == 0
+ || (UINTVAL (operands[2]) + UINTVAL (operands[1])
+ > GET_MODE_BITSIZE (<MODE>mode)))"
+ "bfi\\t%<w>0, %<w>3, %2, %1"
+ [(set_attr "v8type" "bfm")
+ (set_attr "mode" "<MODE>")]
+)
+
(define_insn "*<optab><ALLX:mode>_shft_<GPI:mode>"
[(set (match_operand:GPI 0 "register_operand" "=r")
(ashift:GPI (ANY_EXTEND:GPI
diff --git a/gcc/testsuite/gcc.target/aarch64/insv_1.c b/gcc/testsuite/gcc.target/aarch64/insv_1.c
new file mode 100644
index 0000000..bc8928d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/insv_1.c
@@ -0,0 +1,84 @@
+/* { dg-do run } */
+/* { dg-options "-O2 --save-temps -fno-inline" } */
+
+extern void abort (void);
+
+typedef struct bitfield
+{
+ unsigned short eight: 8;
+ unsigned short four: 4;
+ unsigned short five: 5;
+ unsigned short seven: 7;
+ unsigned int sixteen: 16;
+} bitfield;
+
+bitfield
+bfi1 (bitfield a)
+{
+ /* { dg-final { scan-assembler "bfi\tx\[0-9\]+, x\[0-9\]+, 0, 8" } } */
+ a.eight = 3;
+ return a;
+}
+
+bitfield
+bfi2 (bitfield a)
+{
+ /* { dg-final { scan-assembler "bfi\tx\[0-9\]+, x\[0-9\]+, 16, 5" } } */
+ a.five = 7;
+ return a;
+}
+
+bitfield
+movk (bitfield a)
+{
+ /* { dg-final { scan-assembler "movk\tx\[0-9\]+, 0x1d6b, lsl 32" } } */
+ a.sixteen = 7531;
+ return a;
+}
+
+bitfield
+set1 (bitfield a)
+{
+ /* { dg-final { scan-assembler "orr\tx\[0-9\]+, x\[0-9\]+, 2031616" } } */
+ a.five = 0x1f;
+ return a;
+}
+
+bitfield
+set0 (bitfield a)
+{
+ /* { dg-final { scan-assembler "and\tx\[0-9\]+, x\[0-9\]+, -2031617" } } */
+ a.five = 0;
+ return a;
+}
+
+
+int
+main (int argc, char** argv)
+{
+ static bitfield a;
+ bitfield b = bfi1 (a);
+ bitfield c = bfi2 (b);
+ bitfield d = movk (c);
+
+ if (d.eight != 3)
+ abort ();
+
+ if (d.five != 7)
+ abort ();
+
+ if (d.sixteen != 7531)
+ abort ();
+
+ d = set1 (d);
+ if (d.five != 0x1f)
+ abort ();
+
+ d = set0 (d);
+ if (d.five != 0)
+ abort ();
+
+ return 0;
+}
+
+/* { dg-final { cleanup-saved-temps } } */
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH, AArch64] Support BFI instruction and insv standard pattern
2013-05-30 11:33 ` Ian Bolton
@ 2013-05-30 14:19 ` Richard Henderson
0 siblings, 0 replies; 5+ messages in thread
From: Richard Henderson @ 2013-05-30 14:19 UTC (permalink / raw)
To: Ian Bolton; +Cc: gcc-patches
On 2013-05-30 04:32, Ian Bolton wrote:
>> On 05/20/2013 11:55 AM, Ian Bolton wrote:
>>> > >I improved this patch during the work I did on the recent insv_imm
>> >patch
>>> > >(http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01007.html).
>> >
>> >Thanks, you cleaned up almost everything on which I would have
>> >commented
>> >with the previous patch revision. The only thing left is:
>> >
>>> > >+ else if (!register_operand (value, <MODE>mode))
>>> > >+ operands[3] = force_reg (<MODE>mode, value);
>> >
>> >Checking register_operand before force_reg is unnecessary; you're not
>> >saving a
>> >function call, and force_reg will itself perform the register check.
> Thanks for the review, Richard.
>
> Latest patch is attached, which fixes this.
>
> Linux and bare-metal regression runs successful.
>
Ok.
r~
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-05-30 14:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-08 9:58 [PATCH, AArch64] Support BFI instruction and insv standard pattern Ian Bolton
2013-05-20 18:56 ` Ian Bolton
2013-05-20 19:36 ` Richard Henderson
2013-05-30 11:33 ` Ian Bolton
2013-05-30 14:19 ` Richard Henderson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).