Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-25 Thread Uros Bizjak 
On Mon, Apr 25, 2016 at 9:45 PM, Richard Sandiford
 wrote:

>>> Can you please investigate, what is wrong with all_ones_operand so it
>>> doesn't accept all (-1) operands?
>>
>> Does following work:
>>
>> ;; Return true if operand is a (vector) constant with all bits set.
>> (define_predicate "all_ones_operand"
>>   (match_code "const_int,const_wide_int,const_vector")
>> {
>>   if (op == constm1_rtx)
>> return true;
>>
>>   if (CONST_INT_P (op))
>> return INTVAL (op) == HOST_WIDE_INT_M1;
>>
>>   if (mode == VOIDmode)
>> mode = GET_MODE (op);
>>   return op == CONSTM1_RTX (mode);
>> })
>
> const_wide_int isn't necessary here.  An all-1s integer will always
> use CONST_INT, regardless of the mode size.
>
> I think this reduces to:
>
> (define_predicate "all_ones_operand"
>   (match_code "const_int,const_vector")
> {
>   if (CONST_INT_P (op))
> return INTVAL (op) == HOST_WIDE_INT_M1;
>   return op == CONSTM1_RTX (GET_MODE (op));
> }
>
> (which is still more complex than it should be -- roll on CONST_INTs
> with modes. :-))

This is now implemented in a different way, but nevertheless some
const_wide_int codes were removed from other predicates in a follow-up
patch.

Uros.

Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-25 Thread Richard Sandiford 
Uros Bizjak  writes:
> On Fri, Apr 22, 2016 at 7:10 PM, Uros Bizjak  wrote:
>> On Fri, Apr 22, 2016 at 4:19 PM, H.J. Lu  wrote:
>>> On Fri, Apr 22, 2016 at 5:11 AM, Uros Bizjak  wrote:
 On Thu, Apr 21, 2016 at 10:58 PM, H.J. Lu  wrote:

> Here is the updated patch with my standard_sse_constant_p change and
> your SSE/AVX pattern change.  I didn't include your
> standard_sse_constant_opcode since it didn't compile nor is needed
> for this purpose.

 H.J.,

 please test the attached patch that finally rewrites and improves SSE
 constants handling.

 This is what I want to commit, a follow-up patch will further clean
 standard_sse_constant_opcode wrt TARGET_AVX512VL.

>>>
>>> It doesn't address my problem which is "Allow all 1s of integer as
>>> standard SSE constants".  The key here is "integer".  I'd like to use
>>> SSE/AVX store TI/OI/XI integers with -1.
>>
>> Yes, my patch *should* work for this. Please note that
>> all_ones_operand should catch all cases your additional patch adds.
>>
>> ;; Return true if operand is a (vector) constant with all bits set.
>> (define_predicate "all_ones_operand"
>>   (match_code "const_int,const_wide_int,const_vector")
>> {
>>   if (op == constm1_rtx)
>> return true;
>>
>>   if (mode == VOIDmode)
>> mode = GET_MODE (op);
>>   return op == CONSTM1_RTX (mode);
>> })
>>
>>
>> Can you please investigate, what is wrong with all_ones_operand so it
>> doesn't accept all (-1) operands?
>
> Does following work:
>
> ;; Return true if operand is a (vector) constant with all bits set.
> (define_predicate "all_ones_operand"
>   (match_code "const_int,const_wide_int,const_vector")
> {
>   if (op == constm1_rtx)
> return true;
>
>   if (CONST_INT_P (op))
> return INTVAL (op) == HOST_WIDE_INT_M1;
>
>   if (mode == VOIDmode)
> mode = GET_MODE (op);
>   return op == CONSTM1_RTX (mode);
> })

const_wide_int isn't necessary here.  An all-1s integer will always
use CONST_INT, regardless of the mode size.

I think this reduces to:

(define_predicate "all_ones_operand"
  (match_code "const_int,const_vector")
{
  if (CONST_INT_P (op))
return INTVAL (op) == HOST_WIDE_INT_M1;
  return op == CONSTM1_RTX (GET_MODE (op));
}

(which is still more complex than it should be -- roll on CONST_INTs
with modes. :-))

Thanks,
Richard

Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-24 Thread Uros Bizjak 
Hello!

Attached patch is what I have committed to handle immediates with all
bits set as standard SSE constants.

2016-04-24  Uros Bizjak  
H.J. Lu  

* config/i386/i386-protos.h (standard_sse_constant_p): Add
machine_mode argument.
* config/i386/i386.c (standard_sse_constant_p): Return 2 for
constm1_rtx operands.  For VOIDmode constants, get mode from
pred_mode.  Check mode size if the mode is supported by ABI.
(standard_sse_constant_opcode): Do not use standard_constant_p.
Strictly check ABI support for all-ones operands.
(ix86_legitimate_constant_p): Handle TImode, OImode and XImode
immediates. Update calls to standard_sse_constant_p.
(ix86_expand_vector_move): Update calls to standard_sse_constant_p.
(ix86_rtx_costs): Ditto.
* config/i386/i386.md (*movxi_internal_avx512f): Use
nonimmediate_or_sse_const_operand instead of vector_move_operand.
Use (v,BC) alternative instead of (v,C). Use register_operand
checks instead of MEM_P.
(*movoi_internal_avx): Use nonimmediate_or_sse_const_operand instead
of vector_move_operand.  Add (v,BC) alternative and corresponding avx2
isa attribute.  Use register_operand checks instead of MEM_P.
(*movti_internal): Use nonimmediate_or_sse_const_operand for
TARGET_SSE.  Improve TARGET_SSE insn constraint.  Add (v,BC)
alternative and corresponding sse2 isa attribute.
(*movtf_internal, *movdf_internal, *movsf_interal): Update calls
to standard_sse_constant_p.
(FP constant splitters): Ditto.
* config/i386/constraints.md (BC): Do not use standard_sse_constant_p.
(C): Ditto.
* config/i386/predicates.md (constm1_operand): Remove.
(nonimmediate_or_sse_const_operand): Rewrite using RTX.
* config/i386/sse.md (*_cvtmask2): Use
vector_all_ones_operand instead of constm1_operand.

Patch was bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

Uros.
Index: constraints.md
===
--- constraints.md  (revision 235371)
+++ constraints.md  (working copy)
@@ -186,7 +186,10 @@
 
 (define_constraint "BC"
   "@internal SSE constant operand."
-  (match_test "standard_sse_constant_p (op)"))
+  (and (match_test "TARGET_SSE")
+   (ior (match_test "op == const0_rtx || op == constm1_rtx")
+   (match_operand 0 "const0_operand")
+   (match_operand 0 "vector_all_ones_operand"
 
 ;; Integer constant constraints.
 (define_constraint "I"
@@ -239,7 +242,9 @@
 ;; This can theoretically be any mode's CONST0_RTX.
 (define_constraint "C"
   "SSE constant zero operand."
-  (match_test "standard_sse_constant_p (op) == 1"))
+  (and (match_test "TARGET_SSE")
+   (ior (match_test "op == const0_rtx")
+   (match_operand 0 "const0_operand"
 
 ;; Constant-or-symbol-reference constraints.
 
Index: i386-protos.h
===
--- i386-protos.h   (revision 235371)
+++ i386-protos.h   (working copy)
@@ -50,7 +50,7 @@ extern bool ix86_using_red_zone (void);
 extern int standard_80387_constant_p (rtx);
 extern const char *standard_80387_constant_opcode (rtx);
 extern rtx standard_80387_constant_rtx (int);
-extern int standard_sse_constant_p (rtx);
+extern int standard_sse_constant_p (rtx, machine_mode);
 extern const char *standard_sse_constant_opcode (rtx_insn *, rtx);
 extern bool symbolic_reference_mentioned_p (rtx);
 extern bool extended_reg_mentioned_p (rtx);
Index: i386.c
===
--- i386.c  (revision 235371)
+++ i386.c  (working copy)
@@ -10762,11 +10762,11 @@ standard_80387_constant_rtx (int idx)
   XFmode);
 }
 
-/* Return 1 if X is all 0s and 2 if x is all 1s
+/* Return 1 if X is all bits 0 and 2 if X is all bits 1
in supported SSE/AVX vector mode.  */
 
 int
-standard_sse_constant_p (rtx x)
+standard_sse_constant_p (rtx x, machine_mode pred_mode)
 {
   machine_mode mode;
 
@@ -10774,34 +10774,38 @@ int
 return 0;
 
   mode = GET_MODE (x);
-  
-  if (x == const0_rtx || x == CONST0_RTX (mode))
+
+  if (x == const0_rtx || const0_operand (x, mode))
 return 1;
-  if (vector_all_ones_operand (x, mode))
-switch (mode)
-  {
-  case V16QImode:
-  case V8HImode:
-  case V4SImode:
-  case V2DImode:
-   if (TARGET_SSE2)
- return 2;
-  case V32QImode:
-  case V16HImode:
-  case V8SImode:
-  case V4DImode:
-   if (TARGET_AVX2)
- return 2;
-  case V64QImode:
-  case V32HImode:
-  case V16SImode:
-  case V8DImode:
-   if (TARGET_AVX512F)
- return 2;
-  default:
-   break;
-  }
 
+  if (x == constm1_rtx || vector_all_ones_operand (x, mode))
+{
+  /* VOIDmode integer constant, get mode from the predicate.  */
+  if (mode == VOIDmode)
+   mode = pred_mode;

Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-22 Thread H.J. Lu 
On Fri, Apr 22, 2016 at 11:57 AM, Uros Bizjak <ubiz...@gmail.com> wrote:
> On Fri, Apr 22, 2016 at 8:20 PM, H.J. Lu <hjl.to...@gmail.com> wrote:
>> On Fri, Apr 22, 2016 at 10:29 AM, Uros Bizjak <ubiz...@gmail.com> wrote:
>>> On Fri, Apr 22, 2016 at 7:10 PM, Uros Bizjak <ubiz...@gmail.com> wrote:
>>>> On Fri, Apr 22, 2016 at 4:19 PM, H.J. Lu <hjl.to...@gmail.com> wrote:
>>>>> On Fri, Apr 22, 2016 at 5:11 AM, Uros Bizjak <ubiz...@gmail.com> wrote:
>>>>>> On Thu, Apr 21, 2016 at 10:58 PM, H.J. Lu <hjl.to...@gmail.com> wrote:
>>>>>>
>>>>>>> Here is the updated patch with my standard_sse_constant_p change and
>>>>>>> your SSE/AVX pattern change.  I didn't include your
>>>>>>> standard_sse_constant_opcode since it didn't compile nor is needed
>>>>>>> for this purpose.
>>>>>>
>>>>>> H.J.,
>>>>>>
>>>>>> please test the attached patch that finally rewrites and improves SSE
>>>>>> constants handling.
>>>>>>
>>>>>> This is what I want to commit, a follow-up patch will further clean
>>>>>> standard_sse_constant_opcode wrt TARGET_AVX512VL.
>>>>>>
>>>>>
>>>>> It doesn't address my problem which is "Allow all 1s of integer as
>>>>> standard SSE constants".  The key here is "integer".  I'd like to use
>>>>> SSE/AVX store TI/OI/XI integers with -1.
>>>>
>>>> Yes, my patch *should* work for this. Please note that
>>>> all_ones_operand should catch all cases your additional patch adds.
>>>>
>>>> ;; Return true if operand is a (vector) constant with all bits set.
>>>> (define_predicate "all_ones_operand"
>>>>   (match_code "const_int,const_wide_int,const_vector")
>>>> {
>>>>   if (op == constm1_rtx)
>>>> return true;
>>>>
>>>>   if (mode == VOIDmode)
>>>> mode = GET_MODE (op);
>>>>   return op == CONSTM1_RTX (mode);
>>>> })
>>>>
>>>>
>>>> Can you please investigate, what is wrong with all_ones_operand so it
>>>> doesn't accept all (-1) operands?
>>>
>>> Does following work:
>>>
>>> ;; Return true if operand is a (vector) constant with all bits set.
>>> (define_predicate "all_ones_operand"
>>>   (match_code "const_int,const_wide_int,const_vector")
>>> {
>>>   if (op == constm1_rtx)
>>> return true;
>>>
>>>   if (CONST_INT_P (op))
>>> return INTVAL (op) == HOST_WIDE_INT_M1;
>>>
>>>   if (mode == VOIDmode)
>>> mode = GET_MODE (op);
>>>   return op == CONSTM1_RTX (mode);
>>> })
>>>
>>
>> No.  I need a predicate, all_ones_operand or all_zeros_operand,
>> i.e., standard_sse_constant_p (op, mode) != 0.
>
> The predicate (standard_sse_constant_p) still works this way, but you
> have to provide non-VOID mode in case modeless (-1) is passed. Please
> note that VOID mode with modeless (-1) will ICE by design, since
> standard_sse_constant_p is not able to determine if insn is supported
> by target ISA.
>

This works:

/* Return 1 if X is all bits 0 and 2 if X is all bits 1
   in supported SSE/AVX vector mode.  */

int
standard_sse_constant_p (rtx x, machine_mode pred_mode)
{
  machine_mode mode;

  if (!TARGET_SSE)
return 0;

  if (const0_operand (x, VOIDmode))
return 1;

  mode = GET_MODE (x);

  /* VOIDmode integer constant, infer mode from the predicate.  */
  if (mode == VOIDmode)
mode = pred_mode;

  if (all_ones_operand (x, mode))
 no "else"  ^ mode instead of VOIDmode

This works.

-- 
H.J.
From 3009ef64f7d77f8b7382de1204143a1e2b9e9213 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.to...@gmail.com>
Date: Fri, 22 Apr 2016 06:30:59 -0700
Subject: [PATCH] Allow all 1s of integer as standard SSE constants

---
 gcc/config/i386/constraints.md |   7 ++-
 gcc/config/i386/i386-protos.h  |   2 +-
 gcc/config/i386/i386.c | 136 +
 gcc/config/i386/i386.md|  51 ++--
 gcc/config/i386/predicates.md  |  42 ++---
 gcc/config/i386/sse.md |   4 +-
 6 files changed, 141 insertions(+), 101 deletions(-)

diff --git a/gcc/config/i386/constraints.md b/gcc/config/i386/constraints.md
index afdc546..c02c321 100644
--- a/gcc/config/i386/constraints.md
+++ b/gcc/config/i386/

Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-22 Thread Uros Bizjak 
On Fri, Apr 22, 2016 at 8:20 PM, H.J. Lu  wrote:
> On Fri, Apr 22, 2016 at 10:29 AM, Uros Bizjak  wrote:
>> On Fri, Apr 22, 2016 at 7:10 PM, Uros Bizjak  wrote:
>>> On Fri, Apr 22, 2016 at 4:19 PM, H.J. Lu  wrote:
 On Fri, Apr 22, 2016 at 5:11 AM, Uros Bizjak  wrote:
> On Thu, Apr 21, 2016 at 10:58 PM, H.J. Lu  wrote:
>
>> Here is the updated patch with my standard_sse_constant_p change and
>> your SSE/AVX pattern change.  I didn't include your
>> standard_sse_constant_opcode since it didn't compile nor is needed
>> for this purpose.
>
> H.J.,
>
> please test the attached patch that finally rewrites and improves SSE
> constants handling.
>
> This is what I want to commit, a follow-up patch will further clean
> standard_sse_constant_opcode wrt TARGET_AVX512VL.
>

 It doesn't address my problem which is "Allow all 1s of integer as
 standard SSE constants".  The key here is "integer".  I'd like to use
 SSE/AVX store TI/OI/XI integers with -1.
>>>
>>> Yes, my patch *should* work for this. Please note that
>>> all_ones_operand should catch all cases your additional patch adds.
>>>
>>> ;; Return true if operand is a (vector) constant with all bits set.
>>> (define_predicate "all_ones_operand"
>>>   (match_code "const_int,const_wide_int,const_vector")
>>> {
>>>   if (op == constm1_rtx)
>>> return true;
>>>
>>>   if (mode == VOIDmode)
>>> mode = GET_MODE (op);
>>>   return op == CONSTM1_RTX (mode);
>>> })
>>>
>>>
>>> Can you please investigate, what is wrong with all_ones_operand so it
>>> doesn't accept all (-1) operands?
>>
>> Does following work:
>>
>> ;; Return true if operand is a (vector) constant with all bits set.
>> (define_predicate "all_ones_operand"
>>   (match_code "const_int,const_wide_int,const_vector")
>> {
>>   if (op == constm1_rtx)
>> return true;
>>
>>   if (CONST_INT_P (op))
>> return INTVAL (op) == HOST_WIDE_INT_M1;
>>
>>   if (mode == VOIDmode)
>> mode = GET_MODE (op);
>>   return op == CONSTM1_RTX (mode);
>> })
>>
>
> No.  I need a predicate, all_ones_operand or all_zeros_operand,
> i.e., standard_sse_constant_p (op, mode) != 0.

The predicate (standard_sse_constant_p) still works this way, but you
have to provide non-VOID mode in case modeless (-1) is passed. Please
note that VOID mode with modeless (-1) will ICE by design, since
standard_sse_constant_p is not able to determine if insn is supported
by target ISA.

Uros.

Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-22 Thread H.J. Lu 
On Fri, Apr 22, 2016 at 10:29 AM, Uros Bizjak  wrote:
> On Fri, Apr 22, 2016 at 7:10 PM, Uros Bizjak  wrote:
>> On Fri, Apr 22, 2016 at 4:19 PM, H.J. Lu  wrote:
>>> On Fri, Apr 22, 2016 at 5:11 AM, Uros Bizjak  wrote:
 On Thu, Apr 21, 2016 at 10:58 PM, H.J. Lu  wrote:

> Here is the updated patch with my standard_sse_constant_p change and
> your SSE/AVX pattern change.  I didn't include your
> standard_sse_constant_opcode since it didn't compile nor is needed
> for this purpose.

 H.J.,

 please test the attached patch that finally rewrites and improves SSE
 constants handling.

 This is what I want to commit, a follow-up patch will further clean
 standard_sse_constant_opcode wrt TARGET_AVX512VL.

>>>
>>> It doesn't address my problem which is "Allow all 1s of integer as
>>> standard SSE constants".  The key here is "integer".  I'd like to use
>>> SSE/AVX store TI/OI/XI integers with -1.
>>
>> Yes, my patch *should* work for this. Please note that
>> all_ones_operand should catch all cases your additional patch adds.
>>
>> ;; Return true if operand is a (vector) constant with all bits set.
>> (define_predicate "all_ones_operand"
>>   (match_code "const_int,const_wide_int,const_vector")
>> {
>>   if (op == constm1_rtx)
>> return true;
>>
>>   if (mode == VOIDmode)
>> mode = GET_MODE (op);
>>   return op == CONSTM1_RTX (mode);
>> })
>>
>>
>> Can you please investigate, what is wrong with all_ones_operand so it
>> doesn't accept all (-1) operands?
>
> Does following work:
>
> ;; Return true if operand is a (vector) constant with all bits set.
> (define_predicate "all_ones_operand"
>   (match_code "const_int,const_wide_int,const_vector")
> {
>   if (op == constm1_rtx)
> return true;
>
>   if (CONST_INT_P (op))
> return INTVAL (op) == HOST_WIDE_INT_M1;
>
>   if (mode == VOIDmode)
> mode = GET_MODE (op);
>   return op == CONSTM1_RTX (mode);
> })
>

No.  I need a predicate, all_ones_operand or all_zeros_operand,
i.e., standard_sse_constant_p (op, mode) != 0.


-- 
H.J.

Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-22 Thread Uros Bizjak 
On Fri, Apr 22, 2016 at 7:10 PM, Uros Bizjak  wrote:
> On Fri, Apr 22, 2016 at 4:19 PM, H.J. Lu  wrote:
>> On Fri, Apr 22, 2016 at 5:11 AM, Uros Bizjak  wrote:
>>> On Thu, Apr 21, 2016 at 10:58 PM, H.J. Lu  wrote:
>>>
 Here is the updated patch with my standard_sse_constant_p change and
 your SSE/AVX pattern change.  I didn't include your
 standard_sse_constant_opcode since it didn't compile nor is needed
 for this purpose.
>>>
>>> H.J.,
>>>
>>> please test the attached patch that finally rewrites and improves SSE
>>> constants handling.
>>>
>>> This is what I want to commit, a follow-up patch will further clean
>>> standard_sse_constant_opcode wrt TARGET_AVX512VL.
>>>
>>
>> It doesn't address my problem which is "Allow all 1s of integer as
>> standard SSE constants".  The key here is "integer".  I'd like to use
>> SSE/AVX store TI/OI/XI integers with -1.
>
> Yes, my patch *should* work for this. Please note that
> all_ones_operand should catch all cases your additional patch adds.
>
> ;; Return true if operand is a (vector) constant with all bits set.
> (define_predicate "all_ones_operand"
>   (match_code "const_int,const_wide_int,const_vector")
> {
>   if (op == constm1_rtx)
> return true;
>
>   if (mode == VOIDmode)
> mode = GET_MODE (op);
>   return op == CONSTM1_RTX (mode);
> })
>
>
> Can you please investigate, what is wrong with all_ones_operand so it
> doesn't accept all (-1) operands?

Does following work:

;; Return true if operand is a (vector) constant with all bits set.
(define_predicate "all_ones_operand"
  (match_code "const_int,const_wide_int,const_vector")
{
  if (op == constm1_rtx)
return true;

  if (CONST_INT_P (op))
return INTVAL (op) == HOST_WIDE_INT_M1;

  if (mode == VOIDmode)
mode = GET_MODE (op);
  return op == CONSTM1_RTX (mode);
})


Uros.

Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-22 Thread Uros Bizjak 
On Fri, Apr 22, 2016 at 4:19 PM, H.J. Lu  wrote:
> On Fri, Apr 22, 2016 at 5:11 AM, Uros Bizjak  wrote:
>> On Thu, Apr 21, 2016 at 10:58 PM, H.J. Lu  wrote:
>>
>>> Here is the updated patch with my standard_sse_constant_p change and
>>> your SSE/AVX pattern change.  I didn't include your
>>> standard_sse_constant_opcode since it didn't compile nor is needed
>>> for this purpose.
>>
>> H.J.,
>>
>> please test the attached patch that finally rewrites and improves SSE
>> constants handling.
>>
>> This is what I want to commit, a follow-up patch will further clean
>> standard_sse_constant_opcode wrt TARGET_AVX512VL.
>>
>
> It doesn't address my problem which is "Allow all 1s of integer as
> standard SSE constants".  The key here is "integer".  I'd like to use
> SSE/AVX store TI/OI/XI integers with -1.

Yes, my patch *should* work for this. Please note that
all_ones_operand should catch all cases your additional patch adds.

;; Return true if operand is a (vector) constant with all bits set.
(define_predicate "all_ones_operand"
  (match_code "const_int,const_wide_int,const_vector")
{
  if (op == constm1_rtx)
return true;

  if (mode == VOIDmode)
mode = GET_MODE (op);
  return op == CONSTM1_RTX (mode);
})


Can you please investigate, what is wrong with all_ones_operand so it
doesn't accept all (-1) operands?

Uros.

Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-22 Thread H.J. Lu 
On Fri, Apr 22, 2016 at 7:50 AM, H.J. Lu <hjl.to...@gmail.com> wrote:
> On Fri, Apr 22, 2016 at 7:19 AM, H.J. Lu <hjl.to...@gmail.com> wrote:
>> On Fri, Apr 22, 2016 at 5:11 AM, Uros Bizjak <ubiz...@gmail.com> wrote:
>>> On Thu, Apr 21, 2016 at 10:58 PM, H.J. Lu <hjl.to...@gmail.com> wrote:
>>>
>>>> Here is the updated patch with my standard_sse_constant_p change and
>>>> your SSE/AVX pattern change.  I didn't include your
>>>> standard_sse_constant_opcode since it didn't compile nor is needed
>>>> for this purpose.
>>>
>>> H.J.,
>>>
>>> please test the attached patch that finally rewrites and improves SSE
>>> constants handling.
>>>
>>> This is what I want to commit, a follow-up patch will further clean
>>> standard_sse_constant_opcode wrt TARGET_AVX512VL.
>>>
>>
>> It doesn't address my problem which is "Allow all 1s of integer as
>> standard SSE constants".  The key here is "integer".  I'd like to use
>> SSE/AVX store TI/OI/XI integers with -1.
>>
>> --
>> H.J.
>
> I am testing this on top of yours:
>
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 27c3bbd..677aa71 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -11136,7 +11136,20 @@ standard_sse_constant_p (rtx x, machine_mode 
> pred_mode)
>/* VOIDmode integer constant, infer mode from the predicate.  */
>if (mode == VOIDmode)
>  mode = pred_mode;
> -
> +  if (CONST_INT_P (x))
> +{
> +  /* If mode != VOIDmode, standard_sse_constant_p must be called:
> + 1. On TImode with SSE2.
> + 2. On OImode with AVX2.
> + 3. On XImode with AVX512F.
> +   */
> +  if ((HOST_WIDE_INT) INTVAL (x) == HOST_WIDE_INT_M1
> +  && (mode == VOIDmode
> +  || (mode == TImode && TARGET_SSE2)
> +  || (mode == OImode && TARGET_AVX2)
> +  || (mode == XImode && TARGET_AVX512F)))
> + return 2;
> +}
>else if (all_ones_operand (x, VOIDmode))
>  switch (GET_MODE_SIZE (mode))
>{
>
>

This one works for me.


-- 
H.J.
From fb8c4f9ae238e738d2de5c1f60d741ac01382978 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.to...@gmail.com>
Date: Fri, 22 Apr 2016 06:30:59 -0700
Subject: [PATCH] Allow all 1s of integer as standard SSE constants

Since all 1s in TImode is standard SSE2 constants, all 1s in OImode is
standard AVX2 constants and all 1s in XImode is standard AVX512F constants,
pass mode to standard_sse_constant_p and standard_sse_constant_opcode
to check if all 1s is available for target.

---
 gcc/config/i386/constraints.md |   7 +-
 gcc/config/i386/i386-protos.h  |   2 +-
 gcc/config/i386/i386.c | 149 ++---
 gcc/config/i386/i386.md|  51 --
 gcc/config/i386/predicates.md  |  42 ++--
 gcc/config/i386/sse.md |   4 +-
 6 files changed, 154 insertions(+), 101 deletions(-)

diff --git a/gcc/config/i386/constraints.md b/gcc/config/i386/constraints.md
index afdc546..c02c321 100644
--- a/gcc/config/i386/constraints.md
+++ b/gcc/config/i386/constraints.md
@@ -186,7 +186,9 @@
 
 (define_constraint "BC"
   "@internal SSE constant operand."
-  (match_test "standard_sse_constant_p (op)"))
+  (and (match_test "TARGET_SSE")
+   (ior (match_operand 0 "const0_operand")
+	(match_operand 0 "all_ones_operand"
 
 ;; Integer constant constraints.
 (define_constraint "I"
@@ -239,7 +241,8 @@
 ;; This can theoretically be any mode's CONST0_RTX.
 (define_constraint "C"
   "SSE constant zero operand."
-  (match_test "standard_sse_constant_p (op) == 1"))
+  (and (match_test "TARGET_SSE")
+   (match_operand 0 "const0_operand")))
 
 ;; Constant-or-symbol-reference constraints.
 
diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h
index ff47bc1..93b5e1e 100644
--- a/gcc/config/i386/i386-protos.h
+++ b/gcc/config/i386/i386-protos.h
@@ -50,7 +50,7 @@ extern bool ix86_using_red_zone (void);
 extern int standard_80387_constant_p (rtx);
 extern const char *standard_80387_constant_opcode (rtx);
 extern rtx standard_80387_constant_rtx (int);
-extern int standard_sse_constant_p (rtx);
+extern int standard_sse_constant_p (rtx, machine_mode);
 extern const char *standard_sse_constant_opcode (rtx_insn *, rtx);
 extern bool symbolic_reference_mentioned_p (rtx);
 extern bool extended_reg_mentioned_p (rtx);
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 6379313..4f805e9 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -10762,42 +10762,57 @@ standard_80387_constant_rtx (int idx

Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-22 Thread H.J. Lu 
On Fri, Apr 22, 2016 at 7:19 AM, H.J. Lu  wrote:
> On Fri, Apr 22, 2016 at 5:11 AM, Uros Bizjak  wrote:
>> On Thu, Apr 21, 2016 at 10:58 PM, H.J. Lu  wrote:
>>
>>> Here is the updated patch with my standard_sse_constant_p change and
>>> your SSE/AVX pattern change.  I didn't include your
>>> standard_sse_constant_opcode since it didn't compile nor is needed
>>> for this purpose.
>>
>> H.J.,
>>
>> please test the attached patch that finally rewrites and improves SSE
>> constants handling.
>>
>> This is what I want to commit, a follow-up patch will further clean
>> standard_sse_constant_opcode wrt TARGET_AVX512VL.
>>
>
> It doesn't address my problem which is "Allow all 1s of integer as
> standard SSE constants".  The key here is "integer".  I'd like to use
> SSE/AVX store TI/OI/XI integers with -1.
>
> --
> H.J.

I am testing this on top of yours:

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 27c3bbd..677aa71 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -11136,7 +11136,20 @@ standard_sse_constant_p (rtx x, machine_mode pred_mode)
   /* VOIDmode integer constant, infer mode from the predicate.  */
   if (mode == VOIDmode)
 mode = pred_mode;
-
+  if (CONST_INT_P (x))
+{
+  /* If mode != VOIDmode, standard_sse_constant_p must be called:
+ 1. On TImode with SSE2.
+ 2. On OImode with AVX2.
+ 3. On XImode with AVX512F.
+   */
+  if ((HOST_WIDE_INT) INTVAL (x) == HOST_WIDE_INT_M1
+  && (mode == VOIDmode
+  || (mode == TImode && TARGET_SSE2)
+  || (mode == OImode && TARGET_AVX2)
+  || (mode == XImode && TARGET_AVX512F)))
+ return 2;
+}
   else if (all_ones_operand (x, VOIDmode))
 switch (GET_MODE_SIZE (mode))
   {



-- 
H.J.

Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-22 Thread H.J. Lu 
On Fri, Apr 22, 2016 at 5:11 AM, Uros Bizjak  wrote:
> On Thu, Apr 21, 2016 at 10:58 PM, H.J. Lu  wrote:
>
>> Here is the updated patch with my standard_sse_constant_p change and
>> your SSE/AVX pattern change.  I didn't include your
>> standard_sse_constant_opcode since it didn't compile nor is needed
>> for this purpose.
>
> H.J.,
>
> please test the attached patch that finally rewrites and improves SSE
> constants handling.
>
> This is what I want to commit, a follow-up patch will further clean
> standard_sse_constant_opcode wrt TARGET_AVX512VL.
>

It doesn't address my problem which is "Allow all 1s of integer as
standard SSE constants".  The key here is "integer".  I'd like to use
SSE/AVX store TI/OI/XI integers with -1.

-- 
H.J.

Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-22 Thread Uros Bizjak 
On Thu, Apr 21, 2016 at 10:58 PM, H.J. Lu  wrote:

> Here is the updated patch with my standard_sse_constant_p change and
> your SSE/AVX pattern change.  I didn't include your
> standard_sse_constant_opcode since it didn't compile nor is needed
> for this purpose.

H.J.,

please test the attached patch that finally rewrites and improves SSE
constants handling.

This is what I want to commit, a follow-up patch will further clean
standard_sse_constant_opcode wrt TARGET_AVX512VL.

Bootstrapped and regtested with i386.exp, full regtest in progress.

Uros.
diff --git a/gcc/config/i386/constraints.md b/gcc/config/i386/constraints.md
index afdc546..c02c321 100644
--- a/gcc/config/i386/constraints.md
+++ b/gcc/config/i386/constraints.md
@@ -186,7 +186,9 @@
 
 (define_constraint "BC"
   "@internal SSE constant operand."
-  (match_test "standard_sse_constant_p (op)"))
+  (and (match_test "TARGET_SSE")
+   (ior (match_operand 0 "const0_operand")
+   (match_operand 0 "all_ones_operand"
 
 ;; Integer constant constraints.
 (define_constraint "I"
@@ -239,7 +241,8 @@
 ;; This can theoretically be any mode's CONST0_RTX.
 (define_constraint "C"
   "SSE constant zero operand."
-  (match_test "standard_sse_constant_p (op) == 1"))
+  (and (match_test "TARGET_SSE")
+   (match_operand 0 "const0_operand")))
 
 ;; Constant-or-symbol-reference constraints.
 
diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h
index ff47bc1..93b5e1e 100644
--- a/gcc/config/i386/i386-protos.h
+++ b/gcc/config/i386/i386-protos.h
@@ -50,7 +50,7 @@ extern bool ix86_using_red_zone (void);
 extern int standard_80387_constant_p (rtx);
 extern const char *standard_80387_constant_opcode (rtx);
 extern rtx standard_80387_constant_rtx (int);
-extern int standard_sse_constant_p (rtx);
+extern int standard_sse_constant_p (rtx, machine_mode);
 extern const char *standard_sse_constant_opcode (rtx_insn *, rtx);
 extern bool symbolic_reference_mentioned_p (rtx);
 extern bool extended_reg_mentioned_p (rtx);
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 6379313..874f837 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -10762,42 +10762,44 @@ standard_80387_constant_rtx (int idx)
   XFmode);
 }
 
-/* Return 1 if X is all 0s and 2 if x is all 1s
+/* Return 1 if X is all bits 0 and 2 if X is all bits 1
in supported SSE/AVX vector mode.  */
 
 int
-standard_sse_constant_p (rtx x)
+standard_sse_constant_p (rtx x, machine_mode pred_mode)
 {
   machine_mode mode;
 
   if (!TARGET_SSE)
 return 0;
 
-  mode = GET_MODE (x);
-  
-  if (x == const0_rtx || x == CONST0_RTX (mode))
+  if (const0_operand (x, VOIDmode))
 return 1;
-  if (vector_all_ones_operand (x, mode))
-switch (mode)
+
+  mode = GET_MODE (x);
+
+  /* VOIDmode integer constant, infer mode from the predicate.  */
+  if (mode == VOIDmode)
+mode = pred_mode;
+
+  else if (all_ones_operand (x, VOIDmode))
+switch (GET_MODE_SIZE (mode))
   {
-  case V16QImode:
-  case V8HImode:
-  case V4SImode:
-  case V2DImode:
-   if (TARGET_SSE2)
+  case 64:
+   if (TARGET_AVX512F)
  return 2;
-  case V32QImode:
-  case V16HImode:
-  case V8SImode:
-  case V4DImode:
+   break;
+  case 32:
if (TARGET_AVX2)
  return 2;
-  case V64QImode:
-  case V32HImode:
-  case V16SImode:
-  case V8DImode:
-   if (TARGET_AVX512F)
+   break;
+  case 16:
+   if (TARGET_SSE2)
  return 2;
+   break;
+  case 0:
+   /* VOIDmode */
+   gcc_unreachable ();
   default:
break;
   }
@@ -10811,53 +10813,81 @@ standard_sse_constant_p (rtx x)
 const char *
 standard_sse_constant_opcode (rtx_insn *insn, rtx x)
 {
-  switch (standard_sse_constant_p (x))
+  gcc_assert (TARGET_SSE);
+
+  if (const0_operand (x, VOIDmode))
 {
-case 1:
   switch (get_attr_mode (insn))
{
case MODE_XI:
  return "vpxord\t%g0, %g0, %g0";
-   case MODE_V16SF:
- return TARGET_AVX512DQ ? "vxorps\t%g0, %g0, %g0"
-: "vpxord\t%g0, %g0, %g0";
-   case MODE_V8DF:
- return TARGET_AVX512DQ ? "vxorpd\t%g0, %g0, %g0"
-: "vpxorq\t%g0, %g0, %g0";
+   case MODE_OI:
+ return (TARGET_AVX512VL
+ ? "vpxord\t%x0, %x0, %x0"
+ : "vpxor\t%x0, %x0, %x0");
case MODE_TI:
- return TARGET_AVX512VL ? "vpxord\t%t0, %t0, %t0"
-: "%vpxor\t%0, %d0";
+ return (TARGET_AVX512VL
+ ? "vpxord\t%t0, %t0, %t0"
+ : "%vpxor\t%0, %d0");
+
+   case MODE_V8DF:
+ return (TARGET_AVX512DQ
+ ? "vxorpd\t%g0, %g0, %g0"
+ : "vpxorq\t%g0, %g0, %g0");
+   case MODE_V4DF:
+ return "vxorpd\t%x0, %x0, %x0";
case MODE_V2DF:

Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-21 Thread H.J. Lu 
On Thu, Apr 21, 2016 at 9:37 AM, H.J. Lu <hjl.to...@gmail.com> wrote:
> On Thu, Apr 21, 2016 at 9:31 AM, Uros Bizjak <ubiz...@gmail.com> wrote:
>> On Thu, Apr 21, 2016 at 4:50 PM, H.J. Lu <hjl.to...@gmail.com> wrote:
>>
>>>>> I tried and it doesn't work since the correct mode may not be always
>>>>> available in predicates.  Yes, they pass mode.  But they just do
>>>>>
>>>>> mode = GET_MODE (op);
>>>>>
>>>>> which returns VOIDmode for -1.
>>>>
>>>> Well, looking at generated gcc/insns-preds.c, the predicates do:
>>>>
>>>> (mode == VOIDmode || GET_MODE (op) == mode).
>>>>
>>>> They *check* and don't *assign* "mode" variable.
>>>>
>>>> So, I see no problem checking "mode" variable (that gets the value
>>>> from the pattern) in the predicates.
>>>
>>> This is an incomplete list:
>>>
>>> combine.c:   && ! push_operand (dest, GET_MODE (dest)))
>>> expr.c:  if (push_operand (x, GET_MODE (x)))
>>> expr.c:  && ! push_operand (x, GET_MODE (x
>>> gcse.c:   && ! push_operand (dest, GET_MODE (dest)))
>>> gcse.c:  if (general_operand (exp, GET_MODE (reg)))
>>> ifcvt.c:  if (! general_operand (cmp_a, GET_MODE (cmp_a))
>>> ifcvt.c:  || ! general_operand (cmp_b, GET_MODE (cmp_b)))
>>> ifcvt.c:  else if (general_operand (b, GET_MODE (b)))
>>> ifcvt.c:  if (! general_operand (a, GET_MODE (a)) || tmp_a)
>>> ifcvt.c:  if (! general_operand (b, GET_MODE (b)) || tmp_b)
>>> ira-costs.c:  if (address_operand (op, GET_MODE (op))
>>> ira-costs.c:  && general_operand (SET_SRC (set), GET_MODE (SET_SRC 
>>> (set
>>> lower-subreg.c:  if (GET_MODE (op_operand) != word_mode
>>> lower-subreg.c:  && GET_MODE_SIZE (GET_MODE (op_operand)) > 
>>> UNITS_PER_WORD)
>>> lower-subreg.c: GET_MODE 
>>> (op_operand),
>>> lra-constraints.c: if (simplify_operand_subreg (i, GET_MODE (old)) ||
>>> op_change_p)
>>> optabs.c:  create_output_operand ([0], target, GET_MODE (target));
>>> optabs.c:  create_input_operand ([1], op0, GET_MODE (op0));
>>> postreload-gcse.c:  if (! push_operand (dest, GET_MODE (dest)))
>>> postreload-gcse.c:  && general_operand (src, GET_MODE (src))
>>> postreload-gcse.c:  && general_operand (dest, GET_MODE (dest))
>>> postreload-gcse.c:  && general_operand (src, GET_MODE (src))
>>>
>>> IRA and LRA use GET_MODE and pass it to predicates.
>>
>> I don't know what are you trying to prove here ...
>
> The "mode" argument passed to predates can't be used to determine if
> -1 is a valid SSE constant.
>

Hi Uros,

Here is the updated patch with my standard_sse_constant_p change and
your SSE/AVX pattern change.  I didn't include your
standard_sse_constant_opcode since it didn't compile nor is needed
for this purpose.

-- 
H.J.
From 57c811f25e0735e744d255b3d66a86dbb131749c Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.to...@gmail.com>
Date: Sun, 6 Mar 2016 06:40:31 -0800
Subject: [PATCH] Allow all 1s of integer as standard SSE constants

Since all 1s in TImode is standard SSE2 constants, all 1s in OImode is
standard AVX2 constants and all 1s in XImode is standard AVX512F constants,
pass mode to standard_sse_constant_p and standard_sse_constant_opcode
to check if all 1s is available for target.

	* config/i386/i386-protos.h (standard_sse_constant_p): Take
	machine_mode with VOIDmode as default.
	* config/i386/i386.c (standard_sse_constant_p): Get mode if
	it is VOIDmode.  Return 2 for all 1s of integer in supported
	modes.
	(ix86_expand_vector_move): Pass mode to standard_sse_constant_p.
	* config/i386/i386.md (*movxi_internal_avx512f): Replace
	vector_move_operand with nonimmediate_or_sse_const_operand and
	use BC instead of C in constraint.  Check register_operand
	instead of MEM_P.
	(*movoi_internal_avx): Add a "BC" alternative for AVX2.  Check
	register_operand instead of MEM_P.
	(*movti_internal): Add a "BC" alternative for SSE2.  Check
	register_operand instead of MEM_P for SSE.
---
 gcc/config/i386/i386-protos.h |  2 +-
 gcc/config/i386/i386.c| 27 ---
 gcc/config/i386/i386.md   | 39 ---
 3 files changed, 45 insertions(+), 23 deletions(-)

diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h
index ff47bc1..cf54189 100644
--- a/gcc/config/i386/i386-protos.h
+++ b/gcc/config/i386/i386-protos.h
@@ -50,7 +50,7 @@

Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-21 Thread H.J. Lu 
On Thu, Apr 21, 2016 at 9:31 AM, Uros Bizjak  wrote:
> On Thu, Apr 21, 2016 at 4:50 PM, H.J. Lu  wrote:
>
 I tried and it doesn't work since the correct mode may not be always
 available in predicates.  Yes, they pass mode.  But they just do

 mode = GET_MODE (op);

 which returns VOIDmode for -1.
>>>
>>> Well, looking at generated gcc/insns-preds.c, the predicates do:
>>>
>>> (mode == VOIDmode || GET_MODE (op) == mode).
>>>
>>> They *check* and don't *assign* "mode" variable.
>>>
>>> So, I see no problem checking "mode" variable (that gets the value
>>> from the pattern) in the predicates.
>>
>> This is an incomplete list:
>>
>> combine.c:   && ! push_operand (dest, GET_MODE (dest)))
>> expr.c:  if (push_operand (x, GET_MODE (x)))
>> expr.c:  && ! push_operand (x, GET_MODE (x
>> gcse.c:   && ! push_operand (dest, GET_MODE (dest)))
>> gcse.c:  if (general_operand (exp, GET_MODE (reg)))
>> ifcvt.c:  if (! general_operand (cmp_a, GET_MODE (cmp_a))
>> ifcvt.c:  || ! general_operand (cmp_b, GET_MODE (cmp_b)))
>> ifcvt.c:  else if (general_operand (b, GET_MODE (b)))
>> ifcvt.c:  if (! general_operand (a, GET_MODE (a)) || tmp_a)
>> ifcvt.c:  if (! general_operand (b, GET_MODE (b)) || tmp_b)
>> ira-costs.c:  if (address_operand (op, GET_MODE (op))
>> ira-costs.c:  && general_operand (SET_SRC (set), GET_MODE (SET_SRC 
>> (set
>> lower-subreg.c:  if (GET_MODE (op_operand) != word_mode
>> lower-subreg.c:  && GET_MODE_SIZE (GET_MODE (op_operand)) > 
>> UNITS_PER_WORD)
>> lower-subreg.c: GET_MODE 
>> (op_operand),
>> lra-constraints.c: if (simplify_operand_subreg (i, GET_MODE (old)) ||
>> op_change_p)
>> optabs.c:  create_output_operand ([0], target, GET_MODE (target));
>> optabs.c:  create_input_operand ([1], op0, GET_MODE (op0));
>> postreload-gcse.c:  if (! push_operand (dest, GET_MODE (dest)))
>> postreload-gcse.c:  && general_operand (src, GET_MODE (src))
>> postreload-gcse.c:  && general_operand (dest, GET_MODE (dest))
>> postreload-gcse.c:  && general_operand (src, GET_MODE (src))
>>
>> IRA and LRA use GET_MODE and pass it to predicates.
>
> I don't know what are you trying to prove here ...

The "mode" argument passed to predates can't be used to determine if
-1 is a valid SSE constant.


-- 
H.J.

Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-21 Thread Uros Bizjak 
On Thu, Apr 21, 2016 at 4:50 PM, H.J. Lu  wrote:

>>> I tried and it doesn't work since the correct mode may not be always
>>> available in predicates.  Yes, they pass mode.  But they just do
>>>
>>> mode = GET_MODE (op);
>>>
>>> which returns VOIDmode for -1.
>>
>> Well, looking at generated gcc/insns-preds.c, the predicates do:
>>
>> (mode == VOIDmode || GET_MODE (op) == mode).
>>
>> They *check* and don't *assign* "mode" variable.
>>
>> So, I see no problem checking "mode" variable (that gets the value
>> from the pattern) in the predicates.
>
> This is an incomplete list:
>
> combine.c:   && ! push_operand (dest, GET_MODE (dest)))
> expr.c:  if (push_operand (x, GET_MODE (x)))
> expr.c:  && ! push_operand (x, GET_MODE (x
> gcse.c:   && ! push_operand (dest, GET_MODE (dest)))
> gcse.c:  if (general_operand (exp, GET_MODE (reg)))
> ifcvt.c:  if (! general_operand (cmp_a, GET_MODE (cmp_a))
> ifcvt.c:  || ! general_operand (cmp_b, GET_MODE (cmp_b)))
> ifcvt.c:  else if (general_operand (b, GET_MODE (b)))
> ifcvt.c:  if (! general_operand (a, GET_MODE (a)) || tmp_a)
> ifcvt.c:  if (! general_operand (b, GET_MODE (b)) || tmp_b)
> ira-costs.c:  if (address_operand (op, GET_MODE (op))
> ira-costs.c:  && general_operand (SET_SRC (set), GET_MODE (SET_SRC 
> (set
> lower-subreg.c:  if (GET_MODE (op_operand) != word_mode
> lower-subreg.c:  && GET_MODE_SIZE (GET_MODE (op_operand)) > 
> UNITS_PER_WORD)
> lower-subreg.c: GET_MODE (op_operand),
> lra-constraints.c: if (simplify_operand_subreg (i, GET_MODE (old)) ||
> op_change_p)
> optabs.c:  create_output_operand ([0], target, GET_MODE (target));
> optabs.c:  create_input_operand ([1], op0, GET_MODE (op0));
> postreload-gcse.c:  if (! push_operand (dest, GET_MODE (dest)))
> postreload-gcse.c:  && general_operand (src, GET_MODE (src))
> postreload-gcse.c:  && general_operand (dest, GET_MODE (dest))
> postreload-gcse.c:  && general_operand (src, GET_MODE (src))
>
> IRA and LRA use GET_MODE and pass it to predicates.

I don't know what are you trying to prove here ...

Uros.

Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-21 Thread H.J. Lu 
On Thu, Apr 21, 2016 at 6:59 AM, Uros Bizjak  wrote:
> On Thu, Apr 21, 2016 at 3:54 PM, H.J. Lu  wrote:
>> On Thu, Apr 21, 2016 at 6:48 AM, Uros Bizjak  wrote:
>>> On Thu, Apr 21, 2016 at 3:43 PM, H.J. Lu  wrote:
 On Thu, Apr 21, 2016 at 6:33 AM, Uros Bizjak  wrote:
> On Thu, Apr 21, 2016 at 2:59 PM, H.J. Lu  wrote:
>
>>> We know, that const_int (-1) is allowed with TARGET_SSE2 and that
>>> const_wide_int (-1) is allowed with TARGET_AVX2. Probably we don't
>>> have to check AVX512F in standard_sse_constant_p, as it implies
>>> TARGET_AVX2.
>>>
>>> As said, it is the job of insn mode attributes to emit correct 
>>> instruction.
>>>
>>> Based on the above observations, mode checks for -1 are not needed in
>>> standard_sse_constant_p.
>>
>> void
>> ix86_expand_vector_move (machine_mode mode, rtx operands[])
>> {
>>   rtx op0 = operands[0], op1 = operands[1];
>>   /* Use GET_MODE_BITSIZE instead of GET_MODE_ALIGNMENT for IA MCU
>>  psABI since the biggest alignment is 4 byte for IA MCU psABI.  */
>>   unsigned int align = (TARGET_IAMCU
>> ? GET_MODE_BITSIZE (mode)
>> : GET_MODE_ALIGNMENT (mode));
>>
>>   if (push_operand (op0, VOIDmode))
>> op0 = emit_move_resolve_push (mode, op0);
>>
>>   /* Force constants other than zero into memory.  We do not know how
>>  the instructions used to build constants modify the upper 64 bits
>>  of the register, once we have that information we may be able
>>  to handle some of them more efficiently.  */
>>   if (can_create_pseudo_p ()
>>   && register_operand (op0, mode)
>>   && (CONSTANT_P (op1)
>>   || (SUBREG_P (op1)
>>   && CONSTANT_P (SUBREG_REG (op1
>>   && !standard_sse_constant_p (op1))
>> ^
>>
>> What should it return for  op1 == (VOIDmode) -1 when
>> TARGET_AVX is true and TARGET_AVX2 is false for
>> mode == TImode and mode == OImode?
>>
>> op1 = validize_mem (force_const_mem (mode, op1));
>
> Let me rethink and redesign this whole mess, so we will have
> consistent predicates.

 The problem is because -1 has no mode.  We can't tell
 if -1 is a valid SSE constant without mode.  That is my
 change to standard_sse_constant_p and
 ix86_expand_vector_move is for.   It is sufficient for
 all my tests, including benchmark runs.
>>>
>>> I'm not against mode checks, but IMO, we have to do these checks in
>>> predicates, where we know operand mode.
>>
>> I tried and it doesn't work since the correct mode may not be always
>> available in predicates.  Yes, they pass mode.  But they just do
>>
>> mode = GET_MODE (op);
>>
>> which returns VOIDmode for -1.
>
> Well, looking at generated gcc/insns-preds.c, the predicates do:
>
> (mode == VOIDmode || GET_MODE (op) == mode).
>
> They *check* and don't *assign* "mode" variable.
>
> So, I see no problem checking "mode" variable (that gets the value
> from the pattern) in the predicates.

This is an incomplete list:

combine.c:   && ! push_operand (dest, GET_MODE (dest)))
expr.c:  if (push_operand (x, GET_MODE (x)))
expr.c:  && ! push_operand (x, GET_MODE (x
gcse.c:   && ! push_operand (dest, GET_MODE (dest)))
gcse.c:  if (general_operand (exp, GET_MODE (reg)))
ifcvt.c:  if (! general_operand (cmp_a, GET_MODE (cmp_a))
ifcvt.c:  || ! general_operand (cmp_b, GET_MODE (cmp_b)))
ifcvt.c:  else if (general_operand (b, GET_MODE (b)))
ifcvt.c:  if (! general_operand (a, GET_MODE (a)) || tmp_a)
ifcvt.c:  if (! general_operand (b, GET_MODE (b)) || tmp_b)
ira-costs.c:  if (address_operand (op, GET_MODE (op))
ira-costs.c:  && general_operand (SET_SRC (set), GET_MODE (SET_SRC (set
lower-subreg.c:  if (GET_MODE (op_operand) != word_mode
lower-subreg.c:  && GET_MODE_SIZE (GET_MODE (op_operand)) > UNITS_PER_WORD)
lower-subreg.c: GET_MODE (op_operand),
lra-constraints.c: if (simplify_operand_subreg (i, GET_MODE (old)) ||
op_change_p)
optabs.c:  create_output_operand ([0], target, GET_MODE (target));
optabs.c:  create_input_operand ([1], op0, GET_MODE (op0));
postreload-gcse.c:  if (! push_operand (dest, GET_MODE (dest)))
postreload-gcse.c:  && general_operand (src, GET_MODE (src))
postreload-gcse.c:  && general_operand (dest, GET_MODE (dest))
postreload-gcse.c:  && general_operand (src, GET_MODE (src))

IRA and LRA use GET_MODE and pass it to predicates.

-- 
H.J.

Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-21 Thread Uros Bizjak 
On Thu, Apr 21, 2016 at 3:54 PM, H.J. Lu  wrote:
> On Thu, Apr 21, 2016 at 6:48 AM, Uros Bizjak  wrote:
>> On Thu, Apr 21, 2016 at 3:43 PM, H.J. Lu  wrote:
>>> On Thu, Apr 21, 2016 at 6:33 AM, Uros Bizjak  wrote:
 On Thu, Apr 21, 2016 at 2:59 PM, H.J. Lu  wrote:

>> We know, that const_int (-1) is allowed with TARGET_SSE2 and that
>> const_wide_int (-1) is allowed with TARGET_AVX2. Probably we don't
>> have to check AVX512F in standard_sse_constant_p, as it implies
>> TARGET_AVX2.
>>
>> As said, it is the job of insn mode attributes to emit correct 
>> instruction.
>>
>> Based on the above observations, mode checks for -1 are not needed in
>> standard_sse_constant_p.
>
> void
> ix86_expand_vector_move (machine_mode mode, rtx operands[])
> {
>   rtx op0 = operands[0], op1 = operands[1];
>   /* Use GET_MODE_BITSIZE instead of GET_MODE_ALIGNMENT for IA MCU
>  psABI since the biggest alignment is 4 byte for IA MCU psABI.  */
>   unsigned int align = (TARGET_IAMCU
> ? GET_MODE_BITSIZE (mode)
> : GET_MODE_ALIGNMENT (mode));
>
>   if (push_operand (op0, VOIDmode))
> op0 = emit_move_resolve_push (mode, op0);
>
>   /* Force constants other than zero into memory.  We do not know how
>  the instructions used to build constants modify the upper 64 bits
>  of the register, once we have that information we may be able
>  to handle some of them more efficiently.  */
>   if (can_create_pseudo_p ()
>   && register_operand (op0, mode)
>   && (CONSTANT_P (op1)
>   || (SUBREG_P (op1)
>   && CONSTANT_P (SUBREG_REG (op1
>   && !standard_sse_constant_p (op1))
> ^
>
> What should it return for  op1 == (VOIDmode) -1 when
> TARGET_AVX is true and TARGET_AVX2 is false for
> mode == TImode and mode == OImode?
>
> op1 = validize_mem (force_const_mem (mode, op1));

 Let me rethink and redesign this whole mess, so we will have
 consistent predicates.
>>>
>>> The problem is because -1 has no mode.  We can't tell
>>> if -1 is a valid SSE constant without mode.  That is my
>>> change to standard_sse_constant_p and
>>> ix86_expand_vector_move is for.   It is sufficient for
>>> all my tests, including benchmark runs.
>>
>> I'm not against mode checks, but IMO, we have to do these checks in
>> predicates, where we know operand mode.
>
> I tried and it doesn't work since the correct mode may not be always
> available in predicates.  Yes, they pass mode.  But they just do
>
> mode = GET_MODE (op);
>
> which returns VOIDmode for -1.

Well, looking at generated gcc/insns-preds.c, the predicates do:

(mode == VOIDmode || GET_MODE (op) == mode).

They *check* and don't *assign* "mode" variable.

So, I see no problem checking "mode" variable (that gets the value
from the pattern) in the predicates.

Uros.

Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-21 Thread H.J. Lu 
On Thu, Apr 21, 2016 at 6:48 AM, Uros Bizjak  wrote:
> On Thu, Apr 21, 2016 at 3:43 PM, H.J. Lu  wrote:
>> On Thu, Apr 21, 2016 at 6:33 AM, Uros Bizjak  wrote:
>>> On Thu, Apr 21, 2016 at 2:59 PM, H.J. Lu  wrote:
>>>
> We know, that const_int (-1) is allowed with TARGET_SSE2 and that
> const_wide_int (-1) is allowed with TARGET_AVX2. Probably we don't
> have to check AVX512F in standard_sse_constant_p, as it implies
> TARGET_AVX2.
>
> As said, it is the job of insn mode attributes to emit correct 
> instruction.
>
> Based on the above observations, mode checks for -1 are not needed in
> standard_sse_constant_p.

 void
 ix86_expand_vector_move (machine_mode mode, rtx operands[])
 {
   rtx op0 = operands[0], op1 = operands[1];
   /* Use GET_MODE_BITSIZE instead of GET_MODE_ALIGNMENT for IA MCU
  psABI since the biggest alignment is 4 byte for IA MCU psABI.  */
   unsigned int align = (TARGET_IAMCU
 ? GET_MODE_BITSIZE (mode)
 : GET_MODE_ALIGNMENT (mode));

   if (push_operand (op0, VOIDmode))
 op0 = emit_move_resolve_push (mode, op0);

   /* Force constants other than zero into memory.  We do not know how
  the instructions used to build constants modify the upper 64 bits
  of the register, once we have that information we may be able
  to handle some of them more efficiently.  */
   if (can_create_pseudo_p ()
   && register_operand (op0, mode)
   && (CONSTANT_P (op1)
   || (SUBREG_P (op1)
   && CONSTANT_P (SUBREG_REG (op1
   && !standard_sse_constant_p (op1))
 ^

 What should it return for  op1 == (VOIDmode) -1 when
 TARGET_AVX is true and TARGET_AVX2 is false for
 mode == TImode and mode == OImode?

 op1 = validize_mem (force_const_mem (mode, op1));
>>>
>>> Let me rethink and redesign this whole mess, so we will have
>>> consistent predicates.
>>
>> The problem is because -1 has no mode.  We can't tell
>> if -1 is a valid SSE constant without mode.  That is my
>> change to standard_sse_constant_p and
>> ix86_expand_vector_move is for.   It is sufficient for
>> all my tests, including benchmark runs.
>
> I'm not against mode checks, but IMO, we have to do these checks in
> predicates, where we know operand mode.

I tried and it doesn't work since the correct mode may not be always
available in predicates.  Yes, they pass mode.  But they just do

mode = GET_MODE (op);

which returns VOIDmode for -1.

-- 
H.J.

Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-21 Thread Uros Bizjak 
On Thu, Apr 21, 2016 at 3:43 PM, H.J. Lu  wrote:
> On Thu, Apr 21, 2016 at 6:33 AM, Uros Bizjak  wrote:
>> On Thu, Apr 21, 2016 at 2:59 PM, H.J. Lu  wrote:
>>
 We know, that const_int (-1) is allowed with TARGET_SSE2 and that
 const_wide_int (-1) is allowed with TARGET_AVX2. Probably we don't
 have to check AVX512F in standard_sse_constant_p, as it implies
 TARGET_AVX2.

 As said, it is the job of insn mode attributes to emit correct instruction.

 Based on the above observations, mode checks for -1 are not needed in
 standard_sse_constant_p.
>>>
>>> void
>>> ix86_expand_vector_move (machine_mode mode, rtx operands[])
>>> {
>>>   rtx op0 = operands[0], op1 = operands[1];
>>>   /* Use GET_MODE_BITSIZE instead of GET_MODE_ALIGNMENT for IA MCU
>>>  psABI since the biggest alignment is 4 byte for IA MCU psABI.  */
>>>   unsigned int align = (TARGET_IAMCU
>>> ? GET_MODE_BITSIZE (mode)
>>> : GET_MODE_ALIGNMENT (mode));
>>>
>>>   if (push_operand (op0, VOIDmode))
>>> op0 = emit_move_resolve_push (mode, op0);
>>>
>>>   /* Force constants other than zero into memory.  We do not know how
>>>  the instructions used to build constants modify the upper 64 bits
>>>  of the register, once we have that information we may be able
>>>  to handle some of them more efficiently.  */
>>>   if (can_create_pseudo_p ()
>>>   && register_operand (op0, mode)
>>>   && (CONSTANT_P (op1)
>>>   || (SUBREG_P (op1)
>>>   && CONSTANT_P (SUBREG_REG (op1
>>>   && !standard_sse_constant_p (op1))
>>> ^
>>>
>>> What should it return for  op1 == (VOIDmode) -1 when
>>> TARGET_AVX is true and TARGET_AVX2 is false for
>>> mode == TImode and mode == OImode?
>>>
>>> op1 = validize_mem (force_const_mem (mode, op1));
>>
>> Let me rethink and redesign this whole mess, so we will have
>> consistent predicates.
>
> The problem is because -1 has no mode.  We can't tell
> if -1 is a valid SSE constant without mode.  That is my
> change to standard_sse_constant_p and
> ix86_expand_vector_move is for.   It is sufficient for
> all my tests, including benchmark runs.

I'm not against mode checks, but IMO, we have to do these checks in
predicates, where we know operand mode.

Uros.

Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-21 Thread H.J. Lu 
On Thu, Apr 21, 2016 at 6:33 AM, Uros Bizjak  wrote:
> On Thu, Apr 21, 2016 at 2:59 PM, H.J. Lu  wrote:
>
>>> We know, that const_int (-1) is allowed with TARGET_SSE2 and that
>>> const_wide_int (-1) is allowed with TARGET_AVX2. Probably we don't
>>> have to check AVX512F in standard_sse_constant_p, as it implies
>>> TARGET_AVX2.
>>>
>>> As said, it is the job of insn mode attributes to emit correct instruction.
>>>
>>> Based on the above observations, mode checks for -1 are not needed in
>>> standard_sse_constant_p.
>>
>> void
>> ix86_expand_vector_move (machine_mode mode, rtx operands[])
>> {
>>   rtx op0 = operands[0], op1 = operands[1];
>>   /* Use GET_MODE_BITSIZE instead of GET_MODE_ALIGNMENT for IA MCU
>>  psABI since the biggest alignment is 4 byte for IA MCU psABI.  */
>>   unsigned int align = (TARGET_IAMCU
>> ? GET_MODE_BITSIZE (mode)
>> : GET_MODE_ALIGNMENT (mode));
>>
>>   if (push_operand (op0, VOIDmode))
>> op0 = emit_move_resolve_push (mode, op0);
>>
>>   /* Force constants other than zero into memory.  We do not know how
>>  the instructions used to build constants modify the upper 64 bits
>>  of the register, once we have that information we may be able
>>  to handle some of them more efficiently.  */
>>   if (can_create_pseudo_p ()
>>   && register_operand (op0, mode)
>>   && (CONSTANT_P (op1)
>>   || (SUBREG_P (op1)
>>   && CONSTANT_P (SUBREG_REG (op1
>>   && !standard_sse_constant_p (op1))
>> ^
>>
>> What should it return for  op1 == (VOIDmode) -1 when
>> TARGET_AVX is true and TARGET_AVX2 is false for
>> mode == TImode and mode == OImode?
>>
>> op1 = validize_mem (force_const_mem (mode, op1));
>
> Let me rethink and redesign this whole mess, so we will have
> consistent predicates.

The problem is because -1 has no mode.  We can't tell
if -1 is a valid SSE constant without mode.  That is my
change to standard_sse_constant_p and
ix86_expand_vector_move is for.   It is sufficient for
all my tests, including benchmark runs.


-- 
H.J.

Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-21 Thread Uros Bizjak 
On Thu, Apr 21, 2016 at 2:59 PM, H.J. Lu  wrote:

>> We know, that const_int (-1) is allowed with TARGET_SSE2 and that
>> const_wide_int (-1) is allowed with TARGET_AVX2. Probably we don't
>> have to check AVX512F in standard_sse_constant_p, as it implies
>> TARGET_AVX2.
>>
>> As said, it is the job of insn mode attributes to emit correct instruction.
>>
>> Based on the above observations, mode checks for -1 are not needed in
>> standard_sse_constant_p.
>
> void
> ix86_expand_vector_move (machine_mode mode, rtx operands[])
> {
>   rtx op0 = operands[0], op1 = operands[1];
>   /* Use GET_MODE_BITSIZE instead of GET_MODE_ALIGNMENT for IA MCU
>  psABI since the biggest alignment is 4 byte for IA MCU psABI.  */
>   unsigned int align = (TARGET_IAMCU
> ? GET_MODE_BITSIZE (mode)
> : GET_MODE_ALIGNMENT (mode));
>
>   if (push_operand (op0, VOIDmode))
> op0 = emit_move_resolve_push (mode, op0);
>
>   /* Force constants other than zero into memory.  We do not know how
>  the instructions used to build constants modify the upper 64 bits
>  of the register, once we have that information we may be able
>  to handle some of them more efficiently.  */
>   if (can_create_pseudo_p ()
>   && register_operand (op0, mode)
>   && (CONSTANT_P (op1)
>   || (SUBREG_P (op1)
>   && CONSTANT_P (SUBREG_REG (op1
>   && !standard_sse_constant_p (op1))
> ^
>
> What should it return for  op1 == (VOIDmode) -1 when
> TARGET_AVX is true and TARGET_AVX2 is false for
> mode == TImode and mode == OImode?
>
> op1 = validize_mem (force_const_mem (mode, op1));

Let me rethink and redesign this whole mess, so we will have
consistent predicates.

Uros.

Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-21 Thread H.J. Lu 
On Thu, Apr 21, 2016 at 5:15 AM, Uros Bizjak  wrote:
> On Thu, Apr 21, 2016 at 1:54 PM, H.J. Lu  wrote:
>> On Thu, Apr 21, 2016 at 3:18 AM, Uros Bizjak  wrote:
>>> On Thu, Apr 21, 2016 at 9:42 AM, Uros Bizjak  wrote:
 On Thu, Apr 21, 2016 at 9:37 AM, Uros Bizjak  wrote:
> On Wed, Apr 20, 2016 at 9:53 PM, H.J. Lu  wrote:
>> Since all 1s in TImode is standard SSE2 constants, all 1s in OImode is
>> standard AVX2 constants and all 1s in XImode is standard AVX512F 
>> constants,
>> pass mode to standard_sse_constant_p and standard_sse_constant_opcode
>> to check if all 1s is available for target.
>>
>> Tested on Linux/x86-64.  OK for master?
>
> No.
>
> This patch should use "isa" attribute instead of adding even more
> similar patterns. Also, please leave MEM_P checks, the rare C->m move
> can be easily resolved by IRA.

 Actually, register_operand checks are indeed better, please disregard
 MEM_P recommendation.
>>>
>>> So, something like attached untested RFC proto-patch, that lacks
>>> wide-int handling.
>>>
>>> Uros.
>>
>> +
>> +  else if (CONST_INT_P (x))
>> +{
>> +  if (INTVAL (X) == HOST_WIDE_INT_M1
>> +  && TARGET_SSE2)
>> + return 2;
>> +}
>> +  else if (CONST_WIDE_INT_P (x))
>> +{
>> +  if ( something involving wi::minus-one 
>> +  && TARGET_AVX2)
>> + return 2;
>> +  if (
>> +  && TARGET_AVX512F)
>> + return 2;
>> +}
>> +  else if (vector_all_ones_operand (x, mode))
>>
>> All 1s may not use winde_int.  It has VOIDmode.
>> The mode is passed by
>>
>> @@ -18758,7 +18771,7 @@ ix86_expand_vector_move (machine_mode mode,
>> rtx operands[])
>>&& (CONSTANT_P (op1)
>>|| (SUBREG_P (op1)
>>&& CONSTANT_P (SUBREG_REG (op1
>> -  && !standard_sse_constant_p (op1))
>> +  && !standard_sse_constant_p (op1, mode))
>>  op1 = validize_mem (force_const_mem (mode, op1));
>>
>> This is why I have
>>
>> -standard_sse_constant_p (rtx x)
>> +standard_sse_constant_p (rtx x, machine_mode mode)
>>  {
>> -  machine_mode mode;
>> -
>>if (!TARGET_SSE)
>>  return 0;
>>
>> -  mode = GET_MODE (x);
>> -
>> +  if (mode == VOIDmode)
>> +mode = GET_MODE (x);
>> +
>>
>> since all 1s `x' may have VOIDmode when called from
>> ix86_expand_vector_move if mode isn't passed.
>
> We know, that const_int (-1) is allowed with TARGET_SSE2 and that
> const_wide_int (-1) is allowed with TARGET_AVX2. Probably we don't
> have to check AVX512F in standard_sse_constant_p, as it implies
> TARGET_AVX2.
>
> As said, it is the job of insn mode attributes to emit correct instruction.
>
> Based on the above observations, mode checks for -1 are not needed in
> standard_sse_constant_p.

void
ix86_expand_vector_move (machine_mode mode, rtx operands[])
{
  rtx op0 = operands[0], op1 = operands[1];
  /* Use GET_MODE_BITSIZE instead of GET_MODE_ALIGNMENT for IA MCU
 psABI since the biggest alignment is 4 byte for IA MCU psABI.  */
  unsigned int align = (TARGET_IAMCU
? GET_MODE_BITSIZE (mode)
: GET_MODE_ALIGNMENT (mode));

  if (push_operand (op0, VOIDmode))
op0 = emit_move_resolve_push (mode, op0);

  /* Force constants other than zero into memory.  We do not know how
 the instructions used to build constants modify the upper 64 bits
 of the register, once we have that information we may be able
 to handle some of them more efficiently.  */
  if (can_create_pseudo_p ()
  && register_operand (op0, mode)
  && (CONSTANT_P (op1)
  || (SUBREG_P (op1)
  && CONSTANT_P (SUBREG_REG (op1
  && !standard_sse_constant_p (op1))
^

What should it return for  op1 == (VOIDmode) -1 when
TARGET_AVX is true and TARGET_AVX2 is false for
mode == TImode and mode == OImode?

op1 = validize_mem (force_const_mem (mode, op1));


-- 
H.J.

Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-21 Thread Uros Bizjak 
On Thu, Apr 21, 2016 at 1:54 PM, H.J. Lu  wrote:
> On Thu, Apr 21, 2016 at 3:18 AM, Uros Bizjak  wrote:
>> On Thu, Apr 21, 2016 at 9:42 AM, Uros Bizjak  wrote:
>>> On Thu, Apr 21, 2016 at 9:37 AM, Uros Bizjak  wrote:
 On Wed, Apr 20, 2016 at 9:53 PM, H.J. Lu  wrote:
> Since all 1s in TImode is standard SSE2 constants, all 1s in OImode is
> standard AVX2 constants and all 1s in XImode is standard AVX512F 
> constants,
> pass mode to standard_sse_constant_p and standard_sse_constant_opcode
> to check if all 1s is available for target.
>
> Tested on Linux/x86-64.  OK for master?

 No.

 This patch should use "isa" attribute instead of adding even more
 similar patterns. Also, please leave MEM_P checks, the rare C->m move
 can be easily resolved by IRA.
>>>
>>> Actually, register_operand checks are indeed better, please disregard
>>> MEM_P recommendation.
>>
>> So, something like attached untested RFC proto-patch, that lacks
>> wide-int handling.
>>
>> Uros.
>
> +
> +  else if (CONST_INT_P (x))
> +{
> +  if (INTVAL (X) == HOST_WIDE_INT_M1
> +  && TARGET_SSE2)
> + return 2;
> +}
> +  else if (CONST_WIDE_INT_P (x))
> +{
> +  if ( something involving wi::minus-one 
> +  && TARGET_AVX2)
> + return 2;
> +  if (
> +  && TARGET_AVX512F)
> + return 2;
> +}
> +  else if (vector_all_ones_operand (x, mode))
>
> All 1s may not use winde_int.  It has VOIDmode.
> The mode is passed by
>
> @@ -18758,7 +18771,7 @@ ix86_expand_vector_move (machine_mode mode,
> rtx operands[])
>&& (CONSTANT_P (op1)
>|| (SUBREG_P (op1)
>&& CONSTANT_P (SUBREG_REG (op1
> -  && !standard_sse_constant_p (op1))
> +  && !standard_sse_constant_p (op1, mode))
>  op1 = validize_mem (force_const_mem (mode, op1));
>
> This is why I have
>
> -standard_sse_constant_p (rtx x)
> +standard_sse_constant_p (rtx x, machine_mode mode)
>  {
> -  machine_mode mode;
> -
>if (!TARGET_SSE)
>  return 0;
>
> -  mode = GET_MODE (x);
> -
> +  if (mode == VOIDmode)
> +mode = GET_MODE (x);
> +
>
> since all 1s `x' may have VOIDmode when called from
> ix86_expand_vector_move if mode isn't passed.

We know, that const_int (-1) is allowed with TARGET_SSE2 and that
const_wide_int (-1) is allowed with TARGET_AVX2. Probably we don't
have to check AVX512F in standard_sse_constant_p, as it implies
TARGET_AVX2.

As said, it is the job of insn mode attributes to emit correct instruction.

Based on the above observations, mode checks for -1 are not needed in
standard_sse_constant_p.

Uros.

Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-21 Thread H.J. Lu 
On Thu, Apr 21, 2016 at 3:18 AM, Uros Bizjak  wrote:
> On Thu, Apr 21, 2016 at 9:42 AM, Uros Bizjak  wrote:
>> On Thu, Apr 21, 2016 at 9:37 AM, Uros Bizjak  wrote:
>>> On Wed, Apr 20, 2016 at 9:53 PM, H.J. Lu  wrote:
 Since all 1s in TImode is standard SSE2 constants, all 1s in OImode is
 standard AVX2 constants and all 1s in XImode is standard AVX512F constants,
 pass mode to standard_sse_constant_p and standard_sse_constant_opcode
 to check if all 1s is available for target.

 Tested on Linux/x86-64.  OK for master?
>>>
>>> No.
>>>
>>> This patch should use "isa" attribute instead of adding even more
>>> similar patterns. Also, please leave MEM_P checks, the rare C->m move
>>> can be easily resolved by IRA.
>>
>> Actually, register_operand checks are indeed better, please disregard
>> MEM_P recommendation.
>
> So, something like attached untested RFC proto-patch, that lacks
> wide-int handling.
>
> Uros.

+
+  else if (CONST_INT_P (x))
+{
+  if (INTVAL (X) == HOST_WIDE_INT_M1
+  && TARGET_SSE2)
+ return 2;
+}
+  else if (CONST_WIDE_INT_P (x))
+{
+  if ( something involving wi::minus-one 
+  && TARGET_AVX2)
+ return 2;
+  if (
+  && TARGET_AVX512F)
+ return 2;
+}
+  else if (vector_all_ones_operand (x, mode))

All 1s may not use winde_int.  It has VOIDmode.
The mode is passed by

@@ -18758,7 +18771,7 @@ ix86_expand_vector_move (machine_mode mode,
rtx operands[])
   && (CONSTANT_P (op1)
   || (SUBREG_P (op1)
   && CONSTANT_P (SUBREG_REG (op1
-  && !standard_sse_constant_p (op1))
+  && !standard_sse_constant_p (op1, mode))
 op1 = validize_mem (force_const_mem (mode, op1));

This is why I have

-standard_sse_constant_p (rtx x)
+standard_sse_constant_p (rtx x, machine_mode mode)
 {
-  machine_mode mode;
-
   if (!TARGET_SSE)
 return 0;

-  mode = GET_MODE (x);
-
+  if (mode == VOIDmode)
+mode = GET_MODE (x);
+

since all 1s `x' may have VOIDmode when called from
ix86_expand_vector_move if mode isn't passed.

-- 
H.J.

Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-21 Thread Uros Bizjak 
On Thu, Apr 21, 2016 at 9:42 AM, Uros Bizjak  wrote:
> On Thu, Apr 21, 2016 at 9:37 AM, Uros Bizjak  wrote:
>> On Wed, Apr 20, 2016 at 9:53 PM, H.J. Lu  wrote:
>>> Since all 1s in TImode is standard SSE2 constants, all 1s in OImode is
>>> standard AVX2 constants and all 1s in XImode is standard AVX512F constants,
>>> pass mode to standard_sse_constant_p and standard_sse_constant_opcode
>>> to check if all 1s is available for target.
>>>
>>> Tested on Linux/x86-64.  OK for master?
>>
>> No.
>>
>> This patch should use "isa" attribute instead of adding even more
>> similar patterns. Also, please leave MEM_P checks, the rare C->m move
>> can be easily resolved by IRA.
>
> Actually, register_operand checks are indeed better, please disregard
> MEM_P recommendation.

So, something like attached untested RFC proto-patch, that lacks
wide-int handling.

Uros.
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 0687701..572f5bf 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -10777,7 +10777,23 @@ standard_sse_constant_p (rtx x)
   
   if (x == const0_rtx || x == CONST0_RTX (mode))
 return 1;
-  if (vector_all_ones_operand (x, mode))
+
+  else if (CONST_INT_P (x))
+{
+  if (INTVAL (X) == HOST_WIDE_INT_M1
+ && TARGET_SSE2)
+   return 2;
+}
+  else if (CONST_WIDE_INT_P (x))
+{
+  if ( something involving wi::minus-one 
+ && TARGET_AVX2)
+   return 2;
+  if (
+ && TARGET_AVX512F)
+   return 2;
+}
+  else if (vector_all_ones_operand (x, mode))
 switch (mode)
   {
   case V16QImode:
@@ -10811,53 +10827,70 @@ standard_sse_constant_p (rtx x)
 const char *
 standard_sse_constant_opcode (rtx_insn *insn, rtx x)
 {
+  machine_mode insn_mode = get_attr_mode (insn);
+
   switch (standard_sse_constant_p (x))
 {
 case 1:
-  switch (get_attr_mode (insn))
+  switch (insn_mode)
{
case MODE_XI:
  return "vpxord\t%g0, %g0, %g0";
-   case MODE_V16SF:
- return TARGET_AVX512DQ ? "vxorps\t%g0, %g0, %g0"
-: "vpxord\t%g0, %g0, %g0";
-   case MODE_V8DF:
- return TARGET_AVX512DQ ? "vxorpd\t%g0, %g0, %g0"
-: "vpxorq\t%g0, %g0, %g0";
+   case MODE_OI:
+ return (TARGET_AVX512VL
+ ? "vpxord\t%x0, %x0, %x0"
+ : "vpxor\t%x0, %x0, %x0");
case MODE_TI:
- return TARGET_AVX512VL ? "vpxord\t%t0, %t0, %t0"
-: "%vpxor\t%0, %d0";
-   case MODE_V2DF:
- return "%vxorpd\t%0, %d0";
-   case MODE_V4SF:
- return "%vxorps\t%0, %d0";
+ return (TARGET_AVX512VL
+ ? "vpxord\t%t0, %t0, %t0"
+ : "%vpxor\t%0, %d0");
 
-   case MODE_OI:
- return TARGET_AVX512VL ? "vpxord\t%x0, %x0, %x0"
-: "vpxor\t%x0, %x0, %x0";
+   case MODE_V8DF:
+ return (TARGET_AVX512DQ
+ ? "vxorpd\t%g0, %g0, %g0"
+ : "vpxorq\t%g0, %g0, %g0");
case MODE_V4DF:
  return "vxorpd\t%x0, %x0, %x0";
+   case MODE_V2DF:
+ return "%vxorpd\t%0, %d0";
+
+   case MODE_V16SF:
+ return (TARGET_AVX512DQ
+ ? "vxorps\t%g0, %g0, %g0"
+ : "vpxord\t%g0, %g0, %g0");
case MODE_V8SF:
  return "vxorps\t%x0, %x0, %x0";
+   case MODE_V4SF:
+ return "%vxorps\t%0, %d0";
 
default:
  break;
}
 
 case 2:
-  if (TARGET_AVX512VL
- || get_attr_mode (insn) == MODE_XI
- || get_attr_mode (insn) == MODE_V8DF
- || get_attr_mode (insn) == MODE_V16SF)
-   return "vpternlogd\t{$0xFF, %g0, %g0, %g0|%g0, %g0, %g0, 0xFF}";
-  if (TARGET_AVX)
-   return "vpcmpeqd\t%0, %0, %0";
-  else
-   return "pcmpeqd\t%0, %0";
+  switch (GET_MODE_SIZE (insn_mode))
+   {
+   case 64:
+ gcc_assert (TARGET_AVX512F);
+ return "vpternlogd\t{$0xFF, %g0, %g0, %g0|%g0, %g0, %g0, 0xFF}";
+   case 32:
+ gcc_assert (TARGET_AVX2);
+ return (TARGET_AVX512VL
+ ? "vpternlogd\t{$0xFF, %g0, %g0, %g0|%g0, %g0, %g0, 0xFF}";
+ : "vpcmpeqd\t%0, %0, %0");
+   case 16:
+ gcc_assert (TARGET_SSE2);
+ return (TARGET_AVX512VL
+ ? "vpternlogd\t{$0xFF, %g0, %g0, %g0|%g0, %g0, %g0, 0xFF}";
+ : "pcmpeqd\t%0, %0");
+   default:
+ break;
+   }
 
 default:
   break;
 }
+
   gcc_unreachable ();
 }
 
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 38eb98c..3337968 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -1970,9 +1970,11 @@
(set_attr "length_immediate" "1")])
 
 (define_insn "*movxi_internal_avx512f"
-  [(set (match_operand:XI 0 "nonimmediate_operand" "=v,v

Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-21 Thread Uros Bizjak 
On Thu, Apr 21, 2016 at 9:37 AM, Uros Bizjak  wrote:
> On Wed, Apr 20, 2016 at 9:53 PM, H.J. Lu  wrote:
>> Since all 1s in TImode is standard SSE2 constants, all 1s in OImode is
>> standard AVX2 constants and all 1s in XImode is standard AVX512F constants,
>> pass mode to standard_sse_constant_p and standard_sse_constant_opcode
>> to check if all 1s is available for target.
>>
>> Tested on Linux/x86-64.  OK for master?
>
> No.
>
> This patch should use "isa" attribute instead of adding even more
> similar patterns. Also, please leave MEM_P checks, the rare C->m move
> can be easily resolved by IRA.

Actually, register_operand checks are indeed better, please disregard
MEM_P recommendation.

Uros.

Re: [PATCH] Allow all 1s of integer as standard SSE constants

2016-04-21 Thread Uros Bizjak 
On Wed, Apr 20, 2016 at 9:53 PM, H.J. Lu  wrote:
> Since all 1s in TImode is standard SSE2 constants, all 1s in OImode is
> standard AVX2 constants and all 1s in XImode is standard AVX512F constants,
> pass mode to standard_sse_constant_p and standard_sse_constant_opcode
> to check if all 1s is available for target.
>
> Tested on Linux/x86-64.  OK for master?

No.

This patch should use "isa" attribute instead of adding even more
similar patterns. Also, please leave MEM_P checks, the rare C->m move
can be easily resolved by IRA.

Also, the mode checks should be in the predicate,
standard_sse_constant_p just validates if the constant is allowed by
ISA.

Uros.

> BTW, it will be used to fix
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70155
>
>
> H.J.
> ---
> * config/i386/i386-protos.h (standard_sse_constant_p): Take
> machine_mode with VOIDmode as default.
> * config/i386/i386.c (standard_sse_constant_p): Get mode if
> it is VOIDmode.  Return 2 for all 1s of integer in supported
> modes.
> (ix86_expand_vector_move): Pass mode to standard_sse_constant_p.
> * config/i386/i386.md (*movxi_internal_avx512f): Replace
> vector_move_operand with nonimmediate_or_sse_const_operand and
> use BC instead of C in constraint.  Check register_operand
> instead of MEM_P.  Pass mode to standard_sse_constant_opcode.
> (*movoi_internal_avx): Disabled for TARGET_AVX2.  Check
> register_operand instead of MEM_P.
> (*movoi_internal_avx2): New pattern.
> (*movti_internal_sse): Likewise.
> (*movti_internal): Renamed to ...
> (*movti_internal_sse2): This.  Require SSE2.  Use BC instead of
> C in constraint. Check register_operand instead of MEM_P in
> 32-bit mode.
> ---
>  gcc/config/i386/i386-protos.h |   2 +-
>  gcc/config/i386/i386.c|  27 ---
>  gcc/config/i386/i386.md   | 104 
> --
>  3 files changed, 121 insertions(+), 12 deletions(-)
>
> diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h
> index ff47bc1..cf54189 100644
> --- a/gcc/config/i386/i386-protos.h
> +++ b/gcc/config/i386/i386-protos.h
> @@ -50,7 +50,7 @@ extern bool ix86_using_red_zone (void);
>  extern int standard_80387_constant_p (rtx);
>  extern const char *standard_80387_constant_opcode (rtx);
>  extern rtx standard_80387_constant_rtx (int);
> -extern int standard_sse_constant_p (rtx);
> +extern int standard_sse_constant_p (rtx, machine_mode = VOIDmode);
>  extern const char *standard_sse_constant_opcode (rtx_insn *, rtx);
>  extern bool symbolic_reference_mentioned_p (rtx);
>  extern bool extended_reg_mentioned_p (rtx);
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 6379313..dd951c2 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -10766,18 +10766,31 @@ standard_80387_constant_rtx (int idx)
> in supported SSE/AVX vector mode.  */
>
>  int
> -standard_sse_constant_p (rtx x)
> +standard_sse_constant_p (rtx x, machine_mode mode)
>  {
> -  machine_mode mode;
> -
>if (!TARGET_SSE)
>  return 0;
>
> -  mode = GET_MODE (x);
> -
> +  if (mode == VOIDmode)
> +mode = GET_MODE (x);
> +
>if (x == const0_rtx || x == CONST0_RTX (mode))
>  return 1;
> -  if (vector_all_ones_operand (x, mode))
> +  if (CONST_INT_P (x))
> +{
> +  /* If mode != VOIDmode, standard_sse_constant_p must be called:
> +1. On TImode with SSE2.
> +2. On OImode with AVX2.
> +3. On XImode with AVX512F.
> +   */
> +  if ((HOST_WIDE_INT) INTVAL (x) == HOST_WIDE_INT_M1
> + && (mode == VOIDmode
> + || (mode == TImode && TARGET_SSE2)
> + || (mode == OImode && TARGET_AVX2)
> + || (mode == XImode && TARGET_AVX512F)))
> +   return 2;
> +}
> +  else if (vector_all_ones_operand (x, mode))
>  switch (mode)
>{
>case V16QImode:
> @@ -18758,7 +18771,7 @@ ix86_expand_vector_move (machine_mode mode, rtx 
> operands[])
>&& (CONSTANT_P (op1)
>   || (SUBREG_P (op1)
>   && CONSTANT_P (SUBREG_REG (op1
> -  && !standard_sse_constant_p (op1))
> +  && !standard_sse_constant_p (op1, mode))
>  op1 = validize_mem (force_const_mem (mode, op1));
>
>/* We need to check memory alignment for SSE mode since attribute
> diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
> index babd0a4..75227aa 100644
> --- a/gcc/config/i386/i386.md
> +++ b/gcc/config/i386/i386.md
> @@ -1971,8 +1971,10 @@
>
>  (define_insn "*movxi_internal_avx512f"
>[(set (match_operand:XI 0 "nonimmediate_operand" "=v,v ,m")
> -   (match_operand:XI 1 "vector_move_operand"  "C ,vm,v"))]
> -  "TARGET_AVX512F && !(MEM_P (operands[0]) && MEM_P (operands[1]))"
> +   (match_operand:XI 1 "nonimmediate_or_sse_const_operand" "BC,vm,v"))]
> +  "TARGET_AVX512F
> +   &&

[PATCH] Allow all 1s of integer as standard SSE constants

2016-04-20 Thread H.J. Lu 
Since all 1s in TImode is standard SSE2 constants, all 1s in OImode is
standard AVX2 constants and all 1s in XImode is standard AVX512F constants,
pass mode to standard_sse_constant_p and standard_sse_constant_opcode
to check if all 1s is available for target.

Tested on Linux/x86-64.  OK for master?

BTW, it will be used to fix

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70155


H.J.
---
* config/i386/i386-protos.h (standard_sse_constant_p): Take
machine_mode with VOIDmode as default.
* config/i386/i386.c (standard_sse_constant_p): Get mode if
it is VOIDmode.  Return 2 for all 1s of integer in supported
modes.
(ix86_expand_vector_move): Pass mode to standard_sse_constant_p.
* config/i386/i386.md (*movxi_internal_avx512f): Replace
vector_move_operand with nonimmediate_or_sse_const_operand and
use BC instead of C in constraint.  Check register_operand
instead of MEM_P.  Pass mode to standard_sse_constant_opcode.
(*movoi_internal_avx): Disabled for TARGET_AVX2.  Check
register_operand instead of MEM_P.
(*movoi_internal_avx2): New pattern.
(*movti_internal_sse): Likewise.
(*movti_internal): Renamed to ...
(*movti_internal_sse2): This.  Require SSE2.  Use BC instead of
C in constraint. Check register_operand instead of MEM_P in
32-bit mode.
---
 gcc/config/i386/i386-protos.h |   2 +-
 gcc/config/i386/i386.c|  27 ---
 gcc/config/i386/i386.md   | 104 --
 3 files changed, 121 insertions(+), 12 deletions(-)

diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h
index ff47bc1..cf54189 100644
--- a/gcc/config/i386/i386-protos.h
+++ b/gcc/config/i386/i386-protos.h
@@ -50,7 +50,7 @@ extern bool ix86_using_red_zone (void);
 extern int standard_80387_constant_p (rtx);
 extern const char *standard_80387_constant_opcode (rtx);
 extern rtx standard_80387_constant_rtx (int);
-extern int standard_sse_constant_p (rtx);
+extern int standard_sse_constant_p (rtx, machine_mode = VOIDmode);
 extern const char *standard_sse_constant_opcode (rtx_insn *, rtx);
 extern bool symbolic_reference_mentioned_p (rtx);
 extern bool extended_reg_mentioned_p (rtx);
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 6379313..dd951c2 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -10766,18 +10766,31 @@ standard_80387_constant_rtx (int idx)
in supported SSE/AVX vector mode.  */
 
 int
-standard_sse_constant_p (rtx x)
+standard_sse_constant_p (rtx x, machine_mode mode)
 {
-  machine_mode mode;
-
   if (!TARGET_SSE)
 return 0;
 
-  mode = GET_MODE (x);
-  
+  if (mode == VOIDmode)
+mode = GET_MODE (x);
+
   if (x == const0_rtx || x == CONST0_RTX (mode))
 return 1;
-  if (vector_all_ones_operand (x, mode))
+  if (CONST_INT_P (x))
+{
+  /* If mode != VOIDmode, standard_sse_constant_p must be called:
+1. On TImode with SSE2.
+2. On OImode with AVX2.
+3. On XImode with AVX512F.
+   */
+  if ((HOST_WIDE_INT) INTVAL (x) == HOST_WIDE_INT_M1
+ && (mode == VOIDmode
+ || (mode == TImode && TARGET_SSE2)
+ || (mode == OImode && TARGET_AVX2)
+ || (mode == XImode && TARGET_AVX512F)))
+   return 2;
+}
+  else if (vector_all_ones_operand (x, mode))
 switch (mode)
   {
   case V16QImode:
@@ -18758,7 +18771,7 @@ ix86_expand_vector_move (machine_mode mode, rtx 
operands[])
   && (CONSTANT_P (op1)
  || (SUBREG_P (op1)
  && CONSTANT_P (SUBREG_REG (op1
-  && !standard_sse_constant_p (op1))
+  && !standard_sse_constant_p (op1, mode))
 op1 = validize_mem (force_const_mem (mode, op1));
 
   /* We need to check memory alignment for SSE mode since attribute
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index babd0a4..75227aa 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -1971,8 +1971,10 @@
 
 (define_insn "*movxi_internal_avx512f"
   [(set (match_operand:XI 0 "nonimmediate_operand" "=v,v ,m")
-   (match_operand:XI 1 "vector_move_operand"  "C ,vm,v"))]
-  "TARGET_AVX512F && !(MEM_P (operands[0]) && MEM_P (operands[1]))"
+   (match_operand:XI 1 "nonimmediate_or_sse_const_operand" "BC,vm,v"))]
+  "TARGET_AVX512F
+   && (register_operand (operands[0], XImode)
+   || register_operand (operands[1], XImode))"
 {
   switch (which_alternative)
 {
@@ -1996,7 +1998,10 @@
 (define_insn "*movoi_internal_avx"
   [(set (match_operand:OI 0 "nonimmediate_operand" "=v,v ,m")
(match_operand:OI 1 "vector_move_operand"  "C ,vm,v"))]
-  "TARGET_AVX && !(MEM_P (operands[0]) && MEM_P (operands[1]))"
+  "TARGET_AVX
+   && !TARGET_AVX2
+   && (register_operand (operands[0], OImode)
+   || register_operand (operands[1], OImode))"
 {
   switch (get_attr_type (insn))
 {
@@ -2042,10 +2047,62 @@
  ]