[PATCH] Fix PR target/69810 (GCC 7)

2016-04-28 Thread David Edelsohn
This PR was fixed earlier with a patch that was deemed safe for GCC 6
through the removal of splitters for zero extend and sign extend to
HImode.

Now that trunk has opened for GCC 7 development, the following patch
restores the splitters and fixes the bug in the more aggressive manner
originally proposed: disallow patterns for extension to HImode by
removing HImode from the iterator.  The PowerPC architecture does not
provide any instructions that directly operate on HImode, so it's
better for GCC to operate on it as SUBREG except for loads and stores,
as this patch accomplishes.

Bootstrapped on powerpc-ibm-aix7.1.0.0.

Thanks, David

PR target/69810
* config/rs6000/rs6000.md (EXTQI): Don't allow extension to HImode.
(zero_extendqi2_dot): Revert earlier conversion from
define_insn_and_split to define_insn.
(zero_extendqi2_dot2): Same.
(extendqi2_dot): Same.
(extendqi2_dot2): Same.

Index: rs6000.md
===
--- rs6000.md   (revision 235573)
+++ rs6000.md   (working copy)
@@ -322,7 +322,7 @@
 (define_mode_iterator INT1 [QI HI SI (DI "TARGET_POWERPC64")])

 ; Everything we can extend QImode to.
-(define_mode_iterator EXTQI [HI SI (DI "TARGET_POWERPC64")])
+(define_mode_iterator EXTQI [SI (DI "TARGET_POWERPC64")])

 ; Everything we can extend HImode to.
 (define_mode_iterator EXTHI [SI (DI "TARGET_POWERPC64")])
@@ -711,7 +711,7 @@
rlwinm %0,%1,0,0xff"
   [(set_attr "type" "load,shift")])

-(define_insn "*zero_extendqi2_dot"
+(define_insn_and_split "*zero_extendqi2_dot"
   [(set (match_operand:CC 2 "cc_reg_operand" "=x,?y")
(compare:CC (zero_extend:EXTQI (match_operand:QI 1 "gpc_reg_operand" "r,
r"))
(const_int 0)))
@@ -719,12 +719,19 @@
   "rs6000_gen_cell_microcode"
   "@
andi. %0,%1,0xff
-   rlwinm %0,%1,0,0xff\;cmpwi %2,%0,0"
+   #"
+  "&& reload_completed && cc_reg_not_cr0_operand (operands[2], CCmode)"
+  [(set (match_dup 0)
+   (zero_extend:EXTQI (match_dup 1)))
+   (set (match_dup 2)
+   (compare:CC (match_dup 0)
+   (const_int 0)))]
+  ""
   [(set_attr "type" "logical")
(set_attr "dot" "yes")
(set_attr "length" "4,8")])

-(define_insn "*zero_extendqi2_dot2"
+(define_insn_and_split "*zero_extendqi2_dot2"
   [(set (match_operand:CC 2 "cc_reg_operand" "=x,?y")
(compare:CC (zero_extend:EXTQI (match_operand:QI 1 "gpc_reg_operand" "r,
r"))
(const_int 0)))
@@ -733,7 +740,14 @@
   "rs6000_gen_cell_microcode"
   "@
andi. %0,%1,0xff
-   rlwinm %0,%1,0,0xff\;cmpwi %2,%0,0"
+   #"
+  "&& reload_completed && cc_reg_not_cr0_operand (operands[2], CCmode)"
+  [(set (match_dup 0)
+   (zero_extend:EXTQI (match_dup 1)))
+   (set (match_dup 2)
+   (compare:CC (match_dup 0)
+   (const_int 0)))]
+  ""
   [(set_attr "type" "logical")
(set_attr "dot" "yes")
(set_attr "length" "4,8")])
@@ -851,7 +865,7 @@
   "extsb %0,%1"
   [(set_attr "type" "exts")])

-(define_insn "*extendqi2_dot"
+(define_insn_and_split "*extendqi2_dot"
   [(set (match_operand:CC 2 "cc_reg_operand" "=x,?y")
(compare:CC (sign_extend:EXTQI (match_operand:QI 1 "gpc_reg_operand" "r,
r"))
(const_int 0)))
@@ -859,12 +873,19 @@
   "rs6000_gen_cell_microcode"
   "@
extsb. %0,%1
-   extsb %0,%1\;cmpwi %2,%0,0"
+   #"
+  "&& reload_completed && cc_reg_not_cr0_operand (operands[2], CCmode)"
+  [(set (match_dup 0)
+   (sign_extend:EXTQI (match_dup 1)))
+   (set (match_dup 2)
+   (compare:CC (match_dup 0)
+   (const_int 0)))]
+  ""
   [(set_attr "type" "exts")
(set_attr "dot" "yes")
(set_attr "length" "4,8")])

-(define_insn "*extendqi2_dot2"
+(define_insn_and_split "*extendqi2_dot2"
   [(set (match_operand:CC 2 "cc_reg_operand" "=x,?y")
(compare:CC (sign_extend:EXTQI (match_operand:QI 1 "gpc_reg_operand" "r,
r"))
(const_int 0)))
@@ -873,7 +894,14 @@
   "rs6000_gen_cell_microcode"
   "@
extsb. %0,%1
-   extsb %0,%1\;cmpwi %2,%0,0"
+   #"
+  "&& reload_completed && cc_reg_not_cr0_operand (operands[2], CCmode)"
+  [(set (match_dup 0)
+   (sign_extend:EXTQI (match_dup 1)))
+   (set (match_dup 2)
+   (compare:CC (match_dup 0)
+   (const_int 0)))]
+  ""
   [(set_attr "type" "exts")
(set_attr "dot" "yes")
(set_attr "length" "4,8")])


[PATCH] Fix PR target/69810

2016-02-23 Thread David Edelsohn
Anton reported a latent bug in the rs6000 port discovered with csmith.
Splitters for extendqihi2 and zero_extendqihi2 can generate invalid
compare RTL.  PowerPC can load and store bytes or halfwords, but
computations operate on registers.  Currently the extend patterns
exist for HImode, although no instructions directly operate on
registers in that mode.

For GCC 7, I plan to disable the extendqihi2 and zero_extendqihi2
patterns, forcing GCC to use SUBREGs, but that is too dangerous for
Stage 4.

In the interim, this patch converts the splitters to normal combiner
patterns that directly emit the two instruction sequence when cr0 is
not available.  There is not a lot of scheduling opportunity after the
splitter, so not a huge degradation.  The temporary is allocated as
HImode, but always is a full register.  The compare is hard coded as
cmpw, but the condition bits will be the same for sign-extended or
zero-extended QImode whether the register is interpreted as word or
doubleword.

PR target/69810
* config/rs6000/rs6000.md (zero_extendqi2_dot): Convert from
define_insn_and_split to define_insn.
(zero_extendqi2_dot2): Same.
(extendqi2_dot): Same.
(extendqi2_dot2): Same.

Bootstrapped on powerpc-ibm-aix7.1.0.0 and powerpc64le-linux.

Thanks, David
Index: rs6000.md
===
--- rs6000.md   (revision 232439)
+++ rs6000.md   (working copy)
@@ -701,7 +701,7 @@
rlwinm %0,%1,0,0xff"
   [(set_attr "type" "load,shift")])
 
-(define_insn_and_split "*zero_extendqi2_dot"
+(define_insn "*zero_extendqi2_dot"
   [(set (match_operand:CC 2 "cc_reg_operand" "=x,?y")
(compare:CC (zero_extend:EXTQI (match_operand:QI 1 "gpc_reg_operand" 
"r,r"))
(const_int 0)))
@@ -709,19 +709,12 @@
   "rs6000_gen_cell_microcode"
   "@
andi. %0,%1,0xff
-   #"
-  "&& reload_completed && cc_reg_not_cr0_operand (operands[2], CCmode)"
-  [(set (match_dup 0)
-   (zero_extend:EXTQI (match_dup 1)))
-   (set (match_dup 2)
-   (compare:CC (match_dup 0)
-   (const_int 0)))]
-  ""
+   rlwinm %0,%1,0,0xff\;cmpwi %2,%0,0"
   [(set_attr "type" "logical")
(set_attr "dot" "yes")
(set_attr "length" "4,8")])
 
-(define_insn_and_split "*zero_extendqi2_dot2"
+(define_insn "*zero_extendqi2_dot2"
   [(set (match_operand:CC 2 "cc_reg_operand" "=x,?y")
(compare:CC (zero_extend:EXTQI (match_operand:QI 1 "gpc_reg_operand" 
"r,r"))
(const_int 0)))
@@ -730,14 +723,7 @@
   "rs6000_gen_cell_microcode"
   "@
andi. %0,%1,0xff
-   #"
-  "&& reload_completed && cc_reg_not_cr0_operand (operands[2], CCmode)"
-  [(set (match_dup 0)
-   (zero_extend:EXTQI (match_dup 1)))
-   (set (match_dup 2)
-   (compare:CC (match_dup 0)
-   (const_int 0)))]
-  ""
+   rlwinm %0,%1,0,0xff\;cmpwi %2,%0,0"
   [(set_attr "type" "logical")
(set_attr "dot" "yes")
(set_attr "length" "4,8")])
@@ -855,7 +841,7 @@
   "extsb %0,%1"
   [(set_attr "type" "exts")])
 
-(define_insn_and_split "*extendqi2_dot"
+(define_insn "*extendqi2_dot"
   [(set (match_operand:CC 2 "cc_reg_operand" "=x,?y")
(compare:CC (sign_extend:EXTQI (match_operand:QI 1 "gpc_reg_operand" 
"r,r"))
(const_int 0)))
@@ -863,19 +849,12 @@
   "rs6000_gen_cell_microcode"
   "@
extsb. %0,%1
-   #"
-  "&& reload_completed && cc_reg_not_cr0_operand (operands[2], CCmode)"
-  [(set (match_dup 0)
-   (sign_extend:EXTQI (match_dup 1)))
-   (set (match_dup 2)
-   (compare:CC (match_dup 0)
-   (const_int 0)))]
-  ""
+   extsb %0,%1\;cmpwi %2,%0,0"
   [(set_attr "type" "exts")
(set_attr "dot" "yes")
(set_attr "length" "4,8")])
 
-(define_insn_and_split "*extendqi2_dot2"
+(define_insn "*extendqi2_dot2"
   [(set (match_operand:CC 2 "cc_reg_operand" "=x,?y")
(compare:CC (sign_extend:EXTQI (match_operand:QI 1 "gpc_reg_operand" 
"r,r"))
(const_int 0)))
@@ -884,14 +863,7 @@
   "rs6000_gen_cell_microcode"
   "@
extsb. %0,%1
-   #"
-  "&& reload_completed && cc_reg_not_cr0_operand (operands[2], CCmode)"
-  [(set (match_dup 0)
-   (sign_extend:EXTQI (match_dup 1)))
-   (set (match_dup 2)
-   (compare:CC (match_dup 0)
-   (const_int 0)))]
-  ""
+   extsb %0,%1\;cmpwi %2,%0,0"
   [(set_attr "type" "exts")
(set_attr "dot" "yes")
(set_attr "length" "4,8")])