This is the mail archive of the
gcc@gcc.gnu.org
mailing list for the GCC project.
Re: VIS2 pattern review
- From: David Miller <davem at davemloft dot net>
- To: rth at redhat dot com
- Cc: gcc at gcc dot gnu dot org
- Date: Thu, 13 Oct 2011 15:55:59 -0400 (EDT)
- Subject: Re: VIS2 pattern review
- References: <4E96358F.30405@redhat.com>
From: Richard Henderson <rth@redhat.com>
Date: Wed, 12 Oct 2011 17:49:19 -0700
> The comment for fpmerge_vis is not correct.
> I believe that the operation is representable with
>
> (vec_select:V8QI
> (vec_concat:V8QI
> (match_operand:V4QI 1 ...)
> (match_operand:V4QI 2 ...)
> (parallel [
> 0 4 1 5 2 6 3 7
> ]))
>
> which can be used as the basis for both of the
>
> vec_interleave_lowv8qi
> vec_interleave_highv8qi
>
> named patterns.
Agreed.
> AFAICS, this needs an unspec, like fmul8x16al.
> Similarly for fmul8sux16_vis, fmuld8sux16_vis,
Yes, as we found all the partitioned multiplies need to be unspecs.
>> (define_code_iterator vis3_addsub_ss [ss_plus ss_minus])
>> (define_code_attr vis3_addsub_ss_insn
>> [(ss_plus "fpadds") (ss_minus "fpsubs")])
>>
>> (define_insn "<vis3_addsub_ss_insn><vbits>_vis"
>> [(set (match_operand:VASS 0 "register_operand" "=<vconstr>")
>> (vis3_addsub_ss:VASS (match_operand:VASS 1 "register_operand" "<vconstr>")
>> (match_operand:VASS 2 "register_operand" "<vconstr>")))]
>> "TARGET_VIS3"
>> "<vis3_addsub_ss_insn><vbits>\t%1, %2, %0")
>
> These should be exposed as "ssadd<mode>3" "sssub<mode>3".
Agreed.
I'm currently regstrapping the patch at the end of this mail and will
commit it to trunk if no regressions pop up.
I'll look into the rest of your feedback. But I want to look into a more
fundamental issue with VIS support before moving much further.
I worked for several evenings on adding support for the VIS3
instructions that move directly between float and integer regs. I tried
really hard to get the compiler to do something sensible but it's next
to impossible for two reasons:
1) We don't represent single entry vectors using vector modes, we just
use SImode, DImode etc.
I think this is a huge mistake, because the compiler now thinks it
can do "SImode stuff" in the float regs. Other backends are able
to segregate vector vs. non-vector operations by using the single
entry vector modes.
2) In addition to that, because of how we number the registers for
allocation on sparc for leaf functions, the compiler starts trying
to reload SImode and DImode values into the floating point registers
before trying to use the non-leaf integer regs.
Because the leaf allocation order is "leaf integer regs", "float
regs", "non-leaf integer regs".
Even if I jacked up the register move cost for the cases where the
VIS3 instructions applied, it still did these kinds of reloads.
This also gets reload into trouble because it believes that if it can
move an address value (say Pmode == SImode) from one place to another,
then a plus on the same operands can be performed (with perhaps minor
reloading). But that doesn't work when this "move" is "move float reg
to int reg" and therefore the operands are "%f12" and "%g3". It
tries to do things like "(plus:SI (reg:SI %f12) (reg:SI %g3))"
All of these troubles would be eliminated if we used vector modes for
all the VIS operations instead of using SImode and DImode for the single
entry vector cases.
Unfortunately, that would involve some ABI changes for the VIS
builtins. I'm trending towards considering just changing things
anyways since the VIS intrinsics were next to unusable beforehand.
I've scoured the net for examples of people actually using the GCC
intrinsics before all of my recent changes, and they all fall into two
categories: 1) they use inline assembler because the VIS intrinsics
don't work and 2) they try to use the intrinsics but the code is
disabled because it "doesn't work".
--------------------
Fix the RTL of some sparc VIS patterns.
* config/sparc/sparc.md (UNSPEC_FPMERGE): Delete.
(UNSPEC_MUL16AU, UNSPEC_MUL8, UNSPEC_MUL8SU, UNSPEC_MULDSU): New
unspecs.
(fpmerge_vis): Remove inaccurate comment, represent using vec_select
of a vec_concat.
(vec_interleave_lowv8qi, vec_interleave_highv8qi): New insns.
(fmul8x16_vis, fmul8x16au_vis, fmul8sux16_vis, fmuld8sux16_vis):
Reimplement as unspecs and remove inaccurate comments.
(vis3_shift_patname): New code attr.
(<vis3_shift_insn><vbits>_vis): Rename to "v<vis3_shift_patname><mode>3".
(vis3_addsub_ss_patname): New code attr.
(<vis3_addsub_ss_insn><vbits>_vis): Rename to "<vis3_addsub_ss_patname><mode>".
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 017594f..ae36634 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,5 +1,18 @@
2011-10-12 David S. Miller <davem@davemloft.net>
+ * config/sparc/sparc.md (UNSPEC_FPMERGE): Delete.
+ (UNSPEC_MUL16AU, UNSPEC_MUL8, UNSPEC_MUL8SU, UNSPEC_MULDSU): New
+ unspecs.
+ (fpmerge_vis): Remove inaccurate comment, represent using vec_select
+ of a vec_concat.
+ (vec_interleave_lowv8qi, vec_interleave_highv8qi): New insns.
+ (fmul8x16_vis, fmul8x16au_vis, fmul8sux16_vis, fmuld8sux16_vis):
+ Reimplement as unspecs and remove inaccurate comments.
+ (vis3_shift_patname): New code attr.
+ (<vis3_shift_insn><vbits>_vis): Rename to "v<vis3_shift_patname><mode>3".
+ (vis3_addsub_ss_patname): New code attr.
+ (<vis3_addsub_ss_insn><vbits>_vis): Rename to "<vis3_addsub_ss_patname><mode>".
+
* config/sparc/sparc.h: Do not force TARGET_VIS3 and TARGET_FMAF
to zero when assembler lacks support for such instructions.
* config/sparc/sparc.c (sparc_option_override): Clear MASK_VIS3
diff --git a/gcc/config/sparc/sparc.c b/gcc/config/sparc/sparc.c
index fa790b3..7211658 100644
--- a/gcc/config/sparc/sparc.c
+++ b/gcc/config/sparc/sparc.c
@@ -9492,21 +9492,21 @@ sparc_vis_init_builtins (void)
def_builtin_const ("__builtin_vis_fchksm16", CODE_FOR_fchksm16_vis,
v4hi_ftype_v4hi_v4hi);
- def_builtin_const ("__builtin_vis_fsll16", CODE_FOR_fsll16_vis,
+ def_builtin_const ("__builtin_vis_fsll16", CODE_FOR_vashlv4hi3,
v4hi_ftype_v4hi_v4hi);
- def_builtin_const ("__builtin_vis_fslas16", CODE_FOR_fslas16_vis,
+ def_builtin_const ("__builtin_vis_fslas16", CODE_FOR_vssashlv4hi3,
v4hi_ftype_v4hi_v4hi);
- def_builtin_const ("__builtin_vis_fsrl16", CODE_FOR_fsrl16_vis,
+ def_builtin_const ("__builtin_vis_fsrl16", CODE_FOR_vlshrv4hi3,
v4hi_ftype_v4hi_v4hi);
- def_builtin_const ("__builtin_vis_fsra16", CODE_FOR_fsra16_vis,
+ def_builtin_const ("__builtin_vis_fsra16", CODE_FOR_vashrv4hi3,
v4hi_ftype_v4hi_v4hi);
- def_builtin_const ("__builtin_vis_fsll32", CODE_FOR_fsll32_vis,
+ def_builtin_const ("__builtin_vis_fsll32", CODE_FOR_vashlv2si3,
v2si_ftype_v2si_v2si);
- def_builtin_const ("__builtin_vis_fslas32", CODE_FOR_fslas32_vis,
+ def_builtin_const ("__builtin_vis_fslas32", CODE_FOR_vssashlv2si3,
v2si_ftype_v2si_v2si);
- def_builtin_const ("__builtin_vis_fsrl32", CODE_FOR_fsrl32_vis,
+ def_builtin_const ("__builtin_vis_fsrl32", CODE_FOR_vlshrv2si3,
v2si_ftype_v2si_v2si);
- def_builtin_const ("__builtin_vis_fsra32", CODE_FOR_fsra32_vis,
+ def_builtin_const ("__builtin_vis_fsra32", CODE_FOR_vashrv2si3,
v2si_ftype_v2si_v2si);
if (TARGET_ARCH64)
@@ -9523,21 +9523,21 @@ sparc_vis_init_builtins (void)
def_builtin_const ("__builtin_vis_fpsub64", CODE_FOR_fpsub64_vis,
di_ftype_di_di);
- def_builtin_const ("__builtin_vis_fpadds16", CODE_FOR_fpadds16_vis,
+ def_builtin_const ("__builtin_vis_fpadds16", CODE_FOR_ssaddv4hi,
v4hi_ftype_v4hi_v4hi);
- def_builtin_const ("__builtin_vis_fpadds16s", CODE_FOR_fpadds16s_vis,
+ def_builtin_const ("__builtin_vis_fpadds16s", CODE_FOR_ssaddv2hi,
v2hi_ftype_v2hi_v2hi);
- def_builtin_const ("__builtin_vis_fpsubs16", CODE_FOR_fpsubs16_vis,
+ def_builtin_const ("__builtin_vis_fpsubs16", CODE_FOR_sssubv4hi,
v4hi_ftype_v4hi_v4hi);
- def_builtin_const ("__builtin_vis_fpsubs16s", CODE_FOR_fpsubs16s_vis,
+ def_builtin_const ("__builtin_vis_fpsubs16s", CODE_FOR_sssubv2hi,
v2hi_ftype_v2hi_v2hi);
- def_builtin_const ("__builtin_vis_fpadds32", CODE_FOR_fpadds32_vis,
+ def_builtin_const ("__builtin_vis_fpadds32", CODE_FOR_ssaddv2si,
v2si_ftype_v2si_v2si);
- def_builtin_const ("__builtin_vis_fpadds32s", CODE_FOR_fpadds32s_vis,
+ def_builtin_const ("__builtin_vis_fpadds32s", CODE_FOR_ssaddsi,
v1si_ftype_v1si_v1si);
- def_builtin_const ("__builtin_vis_fpsubs32", CODE_FOR_fpsubs32_vis,
+ def_builtin_const ("__builtin_vis_fpsubs32", CODE_FOR_sssubv2si,
v2si_ftype_v2si_v2si);
- def_builtin_const ("__builtin_vis_fpsubs32s", CODE_FOR_fpsubs32s_vis,
+ def_builtin_const ("__builtin_vis_fpsubs32s", CODE_FOR_sssubsi,
v1si_ftype_v1si_v1si);
if (TARGET_ARCH64)
diff --git a/gcc/config/sparc/sparc.md b/gcc/config/sparc/sparc.md
index 24993fb..fb8fe8b 100644
--- a/gcc/config/sparc/sparc.md
+++ b/gcc/config/sparc/sparc.md
@@ -53,7 +53,7 @@
(UNSPEC_FPACK32 41)
(UNSPEC_FPACKFIX 42)
(UNSPEC_FEXPAND 43)
- (UNSPEC_FPMERGE 44)
+ (UNSPEC_MUL16AU 44)
(UNSPEC_MUL16AL 45)
(UNSPEC_MUL8UL 46)
(UNSPEC_MULDUL 47)
@@ -89,6 +89,9 @@
(UNSPEC_FHADD 83)
(UNSPEC_FHSUB 84)
(UNSPEC_XMUL 85)
+ (UNSPEC_MUL8 86)
+ (UNSPEC_MUL8SU 87)
+ (UNSPEC_MULDSU 88)
])
(define_constants
@@ -8004,36 +8007,64 @@
[(set_attr "type" "fga")
(set_attr "fptype" "double")])
-;; It may be possible to describe this operation as (1 indexed):
-;; (vec_select (vec_duplicate (vec_duplicate (vec_concat 1 2)))
-;; 1,5,10,14,19,23,28,32)
-;; Note that (vec_merge:V8QI [(V4QI) (V4QI)] (10101010 = 170) doesn't work
-;; because vec_merge expects all the operands to be of the same type.
(define_insn "fpmerge_vis"
[(set (match_operand:V8QI 0 "register_operand" "=e")
- (unspec:V8QI [(match_operand:V4QI 1 "register_operand" "f")
- (match_operand:V4QI 2 "register_operand" "f")]
- UNSPEC_FPMERGE))]
+ (vec_select:V8QI
+ (vec_concat:V8QI (match_operand:V4QI 1 "register_operand" "f")
+ (match_operand:V4QI 2 "register_operand" "f"))
+ (parallel [(const_int 0) (const_int 4)
+ (const_int 1) (const_int 5)
+ (const_int 2) (const_int 6)
+ (const_int 3) (const_int 7)])))]
"TARGET_VIS"
"fpmerge\t%1, %2, %0"
[(set_attr "type" "fga")
(set_attr "fptype" "double")])
+(define_insn "vec_interleave_lowv8qi"
+ [(set (match_operand:V8QI 0 "register_operand" "=e")
+ (vec_select:V8QI
+ (vec_concat:V16QI (match_operand:V8QI 1 "register_operand" "f")
+ (match_operand:V8QI 2 "register_operand" "f"))
+ (parallel [(const_int 0) (const_int 8)
+ (const_int 1) (const_int 9)
+ (const_int 2) (const_int 10)
+ (const_int 3) (const_int 11)])))]
+ "TARGET_VIS"
+ "fpmerge\t%L1, %L2, %0"
+ [(set_attr "type" "fga")
+ (set_attr "fptype" "double")])
+
+(define_insn "vec_interleave_highv8qi"
+ [(set (match_operand:V8QI 0 "register_operand" "=e")
+ (vec_select:V8QI
+ (vec_concat:V16QI (match_operand:V8QI 1 "register_operand" "f")
+ (match_operand:V8QI 2 "register_operand" "f"))
+ (parallel [(const_int 4) (const_int 12)
+ (const_int 5) (const_int 13)
+ (const_int 6) (const_int 14)
+ (const_int 7) (const_int 15)])))]
+ "TARGET_VIS"
+ "fpmerge\t%H1, %H2, %0"
+ [(set_attr "type" "fga")
+ (set_attr "fptype" "double")])
+
;; Partitioned multiply instructions
(define_insn "fmul8x16_vis"
[(set (match_operand:V4HI 0 "register_operand" "=e")
- (mult:V4HI (match_operand:V4QI 1 "register_operand" "f")
- (match_operand:V4HI 2 "register_operand" "e")))]
+ (unspec:V4HI [(match_operand:V4QI 1 "register_operand" "f")
+ (match_operand:V4HI 2 "register_operand" "e")]
+ UNSPEC_MUL8))]
"TARGET_VIS"
"fmul8x16\t%1, %2, %0"
[(set_attr "type" "fpmul")
(set_attr "fptype" "double")])
-;; Only one of the following two insns can be a multiply.
(define_insn "fmul8x16au_vis"
[(set (match_operand:V4HI 0 "register_operand" "=e")
- (mult:V4HI (match_operand:V4QI 1 "register_operand" "f")
- (match_operand:V2HI 2 "register_operand" "f")))]
+ (unspec:V4HI [(match_operand:V4QI 1 "register_operand" "f")
+ (match_operand:V2HI 2 "register_operand" "f")]
+ UNSPEC_MUL16AU))]
"TARGET_VIS"
"fmul8x16au\t%1, %2, %0"
[(set_attr "type" "fpmul")
@@ -8049,11 +8080,11 @@
[(set_attr "type" "fpmul")
(set_attr "fptype" "double")])
-;; Only one of the following two insns can be a multiply.
(define_insn "fmul8sux16_vis"
[(set (match_operand:V4HI 0 "register_operand" "=e")
- (mult:V4HI (match_operand:V8QI 1 "register_operand" "e")
- (match_operand:V4HI 2 "register_operand" "e")))]
+ (unspec:V4HI [(match_operand:V8QI 1 "register_operand" "e")
+ (match_operand:V4HI 2 "register_operand" "e")]
+ UNSPEC_MUL8SU))]
"TARGET_VIS"
"fmul8sux16\t%1, %2, %0"
[(set_attr "type" "fpmul")
@@ -8069,11 +8100,11 @@
[(set_attr "type" "fpmul")
(set_attr "fptype" "double")])
-;; Only one of the following two insns can be a multiply.
(define_insn "fmuld8sux16_vis"
[(set (match_operand:V2SI 0 "register_operand" "=e")
- (mult:V2SI (match_operand:V4QI 1 "register_operand" "f")
- (match_operand:V2HI 2 "register_operand" "f")))]
+ (unspec:V2SI [(match_operand:V4QI 1 "register_operand" "f")
+ (match_operand:V2HI 2 "register_operand" "f")]
+ UNSPEC_MULDSU))]
"TARGET_VIS"
"fmuld8sux16\t%1, %2, %0"
[(set_attr "type" "fpmul")
@@ -8440,8 +8471,10 @@
(define_code_iterator vis3_shift [ashift ss_ashift lshiftrt ashiftrt])
(define_code_attr vis3_shift_insn
[(ashift "fsll") (ss_ashift "fslas") (lshiftrt "fsrl") (ashiftrt "fsra")])
+(define_code_attr vis3_shift_patname
+ [(ashift "ashl") (ss_ashift "ssashl") (lshiftrt "lshr") (ashiftrt "ashr")])
-(define_insn "<vis3_shift_insn><vbits>_vis"
+(define_insn "v<vis3_shift_patname><mode>3"
[(set (match_operand:V64N8 0 "register_operand" "=<vconstr>")
(vis3_shift:V64N8 (match_operand:V64N8 1 "register_operand" "<vconstr>")
(match_operand:V64N8 2 "register_operand" "<vconstr>")))]
@@ -8490,8 +8523,10 @@
(define_code_iterator vis3_addsub_ss [ss_plus ss_minus])
(define_code_attr vis3_addsub_ss_insn
[(ss_plus "fpadds") (ss_minus "fpsubs")])
+(define_code_attr vis3_addsub_ss_patname
+ [(ss_plus "ssadd") (ss_minus "sssub")])
-(define_insn "<vis3_addsub_ss_insn><vbits>_vis"
+(define_insn "<vis3_addsub_ss_patname><mode>"
[(set (match_operand:VASS 0 "register_operand" "=<vconstr>")
(vis3_addsub_ss:VASS (match_operand:VASS 1 "register_operand" "<vconstr>")
(match_operand:VASS 2 "register_operand" "<vconstr>")))]