Hi Andre,
On 09/11/16 10:12, Andre Vieira (lists) wrote:
Hi,
This patch implements support for the ARM ACLE Coprocessor LDC and STC
intrinsics. See below a table mapping the intrinsics to their respective
instructions:
+----------------------------------------------------+--------------------------------------+
| Intrinsic signature | Instruction
pattern |
+----------------------------------------------------+--------------------------------------+
|void __arm_ldc(coproc, CRd, const void* p) |LDC coproc, CRd,
[...] |
+----------------------------------------------------+--------------------------------------+
|void __arm_ldcl(coproc, CRd, const void* p) |LDCL coproc, CRd,
[...] |
+----------------------------------------------------+--------------------------------------+
|void __arm_ldc2(coproc, CRd, const void* p) |LDC2 coproc, CRd,
[...] |
+----------------------------------------------------+--------------------------------------+
|void __arm_ldc2l(coproc, CRd, const void* p) |LDC2L coproc, CRd,
[...] |
+----------------------------------------------------+--------------------------------------+
|void __arm_stc(coproc, CRd, void* p) |STC coproc, CRd,
[...] |
+----------------------------------------------------+--------------------------------------+
|void __arm_stcl(coproc, CRd, void* p) |STCL coproc, CRd,
[...] |
+----------------------------------------------------+--------------------------------------+
|void __arm_stc2(coproc, CRd, void* p) |STC2 coproc, CRd,
[...] |
+----------------------------------------------------+--------------------------------------+
|void __arm_stc2l(coproc, CRd, void* p) |STC2L coproc, CRd,
[...] |
+----------------------------------------------------+--------------------------------------+
Note that any untyped variable in the intrinsic signature is required to
be a compiler-time constant and has the type 'unsigned int'. We do some
boundary checks for coproc:[0-15], CR*:[0-31]. If either of these
requirements are not met a diagnostic is issued.
Is this ok for trunk?
I have a few comments below...
Regards,
Andre
gcc/ChangeLog:
2016-11-09 Andre Vieira <andre.simoesdiasvieira@arm.com>
* config/arm/arm.md (*ldcstc): New.
(<ldcstc>): New.
* config/arm/arm.c (arm_coproc_builtin_available): Add
support for ldc,ldcl,stc,stcl,ldc2,ldc2l,stc2 and stc2l.
(arm_coproc_ldc_stc_legitimate_address): New.
* config/arm/arm-builtins.c (arm_type_qualifiers): Add
'qualifier_const_pointer'.
(LDC_QUALIFIERS): Define to...
(arm_ldc_qualifiers): ... this. New.
(STC_QUALIFIERS): Define to...
(arm_stc_qualifiers): ... this. New.
* config/arm/arm-protos.h
(arm_coproc_ldc_stc_legitimate_address): New.
* config/arm/arm_acle.h (__arm_ldc, __arm_ldcl, __arm_stc,
__arm_stcl, __arm_ldc2, __arm_ldc2l, __arm_stc2, __arm_stc2l): New.
* config/arm/arm_acle_builtins.def (ldc, ldc2, ldcl, ldc2l, stc,
stc2, stcl, stc2l): New.
* config/arm/constraints.md (Uz): New.
* config/arm/iterators.md (LDCSTCI, ldcstc, LDCSTC): New.
* config/arm/unspecs.md (VUNSPEC_LDC, VUNSPEC_LDC2, VUNSPEC_LDCL,
VUNSPEC_LDC2L, VUNSPEC_STC, VUNSPEC_STC2, VUNSPEC_STCL,
VUNSPEC_STC2L): New.
gcc/testsuite/ChangeLog:
2016-11-09 Andre Vieira <andre.simoesdiasvieira@arm.com>
* gcc.target/arm/acle/ldc: New.
* gcc.target/arm/acle/ldc2: New.
* gcc.target/arm/acle/ldcl: New.
* gcc.target/arm/acle/ldc2l: New.
* gcc.target/arm/acle/stc: New.
* gcc.target/arm/acle/stc2: New.
* gcc.target/arm/acle/stcl: New.
* gcc.target/arm/acle/stc2l: New.
+ /* const T * foo */
+ qualifier_const_pointer = 0x6,
Nit: full stop at end of comment.
+
+/* This function returns true if OP is a valid memory operand for the
ldc and
+ stc coprocessor instructions and false otherwise. */
+
+bool arm_coproc_ldc_stc_legitimate_address (rtx op)
+{
type and function name should be on separate lines.
+ int range;
+ /* Has to be a memory operand. */
+ if (!MEM_P (op))
+ return false;
+ /* Within the range of [-1020,1020]. */
+ if (range < -1020 || range > 1020)
+ return false;
Use !IN_RANGE (-1020, 1020), also make 'range' a HOST_WIDE_INT (that is
what INTVAL returns).
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -11858,6 +11858,25 @@
[(set_attr "length" "4")
(set_attr "type" "coproc")])
+(define_insn "*ldcstc"
+ [(unspec_volatile [(match_operand:SI 0 "immediate_operand")
+ (match_operand:SI 1 "immediate_operand")
+ (match_operand:SI 2 "memory_operand" "Uz")] LDCSTCI)]
+ "arm_coproc_builtin_available (VUNSPEC_<LDCSTC>)"
You're missing constraints for operands 1 and 2? (just the general 'n'
constraint should do it since you do bounds checking in arm_const_bounds)
Also, the ldc reads memory, whereas stc writes to it, so at least the
stc variants will need
a '=' in their constraint string to tell LRA that the memory is written
to. For this I think it's better to split ldc* and stc* into separate
define_insns.
+(define_memory_constraint "Uz"
+ "@internal
+ A memory access that is accessible"
+ (and (match_code "mem")
+ (match_test "arm_coproc_ldc_stc_legitimate_address (op)")))
That description doesn't make much sense. Did you mean "A memory access
suitable as an LDC/STC operand" ?