[i386] New lea patterns for PR71321

[i386] New lea patterns for PR71321

[Bernd Schmidt]

[-- Attachment #1: Type: text/plain, Size: 597 bytes --]

The problem here is that we don't have complete coverage of lea patterns 
for HImode/QImode: the combiner can't recognize a (plus (ashift reg 2) 
reg) pattern it builds.

My first idea was to canonicalize ASHIFT by constant inside PLUS to 
MULT. The docs say that this is done only inside a MEM context, but that 
seems misguided considering the existence of lea patterns. Maybe that's 
something to revisit for gcc-8. In the meantime, the patch below is more 
like a minimal fix, and it seems to produce better results at the moment 
anyway.

Bootstrapped and tested on x86_64-linux. Ok?


Bernd

[-- Attachment #2: 71321.diff --]
[-- Type: text/x-patch, Size: 3831 bytes --]

	PR target/71321
	* config/i386/i386.md (lea<mode>_general_2b, lea<mode>_general_3b): New
	patterns.
	* config/i386/predicates.md (const123_operand): New.

	PR target/71321
	* gcc.target/i386/pr71321.c: New test.

Index: gcc/config/i386/i386.md
===================================================================
--- gcc/config/i386/i386.md	(revision 243548)
+++ gcc/config/i386/i386.md	(working copy)
@@ -6266,6 +6266,27 @@ (define_insn_and_split "*lea<mode>_gener
   [(set_attr "type" "lea")
    (set_attr "mode" "SI")])
 
+(define_insn_and_split "*lea<mode>_general_2b"
+  [(set (match_operand:SWI12 0 "register_operand" "=r")
+	(plus:SWI12
+	  (ashift:SWI12 (match_operand:SWI12 1 "index_register_operand" "l")
+			(match_operand 2 "const123_operand" "n"))
+	  (match_operand:SWI12 3 "nonmemory_operand" "ri")))]
+  "!TARGET_PARTIAL_REG_STALL || optimize_function_for_size_p (cfun)"
+  "#"
+  "&& reload_completed"
+  [(set (match_dup 0)
+	(plus:SI
+	  (ashift:SI (match_dup 1) (match_dup 2))
+	  (match_dup 3)))]
+{
+  operands[0] = gen_lowpart (SImode, operands[0]);
+  operands[1] = gen_lowpart (SImode, operands[1]);
+  operands[3] = gen_lowpart (SImode, operands[3]);
+}
+  [(set_attr "type" "lea")
+   (set_attr "mode" "SI")])
+
 (define_insn_and_split "*lea<mode>_general_3"
   [(set (match_operand:SWI12 0 "register_operand" "=r")
 	(plus:SWI12
@@ -6284,6 +6305,32 @@ (define_insn_and_split "*lea<mode>_gener
 	    (match_dup 3))
 	  (match_dup 4)))]
 {
+  operands[0] = gen_lowpart (SImode, operands[0]);
+  operands[1] = gen_lowpart (SImode, operands[1]);
+  operands[3] = gen_lowpart (SImode, operands[3]);
+  operands[4] = gen_lowpart (SImode, operands[4]);
+}
+  [(set_attr "type" "lea")
+   (set_attr "mode" "SI")])
+
+(define_insn_and_split "*lea<mode>_general_3b"
+  [(set (match_operand:SWI12 0 "register_operand" "=r")
+	(plus:SWI12
+	  (plus:SWI12
+	    (ashift:SWI12 (match_operand:SWI12 1 "index_register_operand" "l")
+			  (match_operand 2 "const123_operand" "n"))
+	    (match_operand:SWI12 3 "register_operand" "r"))
+	  (match_operand:SWI12 4 "immediate_operand" "i")))]
+  "!TARGET_PARTIAL_REG_STALL || optimize_function_for_size_p (cfun)"
+  "#"
+  "&& reload_completed"
+  [(set (match_dup 0)
+	(plus:SI
+	  (plus:SI
+	    (ashift:SI (match_dup 1) (match_dup 2))
+	    (match_dup 3))
+	  (match_dup 4)))]
+{
   operands[0] = gen_lowpart (SImode, operands[0]);
   operands[1] = gen_lowpart (SImode, operands[1]);
   operands[3] = gen_lowpart (SImode, operands[3]);
Index: gcc/config/i386/predicates.md
===================================================================
--- gcc/config/i386/predicates.md	(revision 243548)
+++ gcc/config/i386/predicates.md	(working copy)
@@ -765,6 +765,14 @@ (define_predicate "const248_operand"
   return i == 2 || i == 4 || i == 8;
 })
 
+;; Match 1, 2, or 3.  Used for lea shift amounts.
+(define_predicate "const123_operand"
+  (match_code "const_int")
+{
+  HOST_WIDE_INT i = INTVAL (op);
+  return i == 1 || i == 2 || i == 3;
+})
+
 ;; Match 2, 3, 6, or 7
 (define_predicate "const2367_operand"
   (match_code "const_int")
Index: gcc/testsuite/gcc.target/i386/pr71321.c
===================================================================
--- gcc/testsuite/gcc.target/i386/pr71321.c	(nonexistent)
+++ gcc/testsuite/gcc.target/i386/pr71321.c	(working copy)
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+typedef unsigned char uint8_t;
+typedef unsigned int uint32_t;
+
+unsigned cvt_to_2digit(uint8_t i, uint8_t base)
+{
+  return ((i / base) | (uint32_t)(i % base)<<8);
+}
+unsigned cvt_to_2digit_ascii(uint8_t i)
+{
+  return cvt_to_2digit(i, 10) + 0x0a3030;
+}
+/* { dg-final { scan-assembler-times "lea.\t\\(%\[0-9a-z\]+,%\[0-9a-z\]+,4" 3 } } */
+/* { dg-final { scan-assembler-times "lea.\t\\(%\[0-9a-z\]+,%\[0-9a-z\]+,8" 1 } } */

Re: [i386] New lea patterns for PR71321

[Uros Bizjak]

On Tue, Dec 20, 2016 at 10:29 PM, Bernd Schmidt <bschmidt@redhat.com> wrote:
> The problem here is that we don't have complete coverage of lea patterns for
> HImode/QImode: the combiner can't recognize a (plus (ashift reg 2) reg)
> pattern it builds.
>
> My first idea was to canonicalize ASHIFT by constant inside PLUS to MULT.
> The docs say that this is done only inside a MEM context, but that seems
> misguided considering the existence of lea patterns. Maybe that's something
> to revisit for gcc-8. In the meantime, the patch below is more like a
> minimal fix, and it seems to produce better results at the moment anyway.

Actually, as mentioned in ix86_decompose_address, we have to handle
ASHIFT anyway, since combine is quite creative when creating various
combinations of PLUS, MULT and ASHIFT in the LEA context. But since
ASHIFT handling is relatively minor functional addition, we just
handle everything in the decomposition, with the side effect that
combine sometimes doesn't canonicalize the combinations involving
ASHIFT.

> Bootstrapped and tested on x86_64-linux. Ok?

OK.

Thanks,
Uros.