On Fri, Dec 4, 2020 at 7:26 PM Segher Boessenkool
<seg...@kernel.crashing.org> wrote:
>
> Hi!
>
> On Fri, Dec 04, 2020 at 07:06:45PM +0100, Uros Bizjak wrote:
> > On Fri, Dec 4, 2020 at 6:57 PM Jakub Jelinek <ja...@redhat.com> wrote:
> > >
> > > On Fri, Dec 04, 2020 at 06:53:49PM +0100, Uros Bizjak wrote:
> > > > > > I was trying that first, but it didn't work. Without the
> > > > > > clobber it actually works right, we don't have the rotate insn with
> > > > > > the
> > > > > > masking and no clobber, so in the end combiner does add the clobber
> > > > > > there
> > > > > > (or would fail it the clobber couldn't be added).
> > > > >
> > > > > I was not aware of that detail ...
> > > >
> > > > That said, IMO, it would be better to rewrite other _mask and _mask_1
> > > > patterns that remove useless masking to combine splitter.
> > > > Unfortunately, the combine splitter expects exactly two output
> > > > instructions for some reason, but these patterns split to one
> > > > instruction. Perhaps it is possible to relax this limitation of
> > > > combine splitters and also allow one output instruction.
> > >
> > > I've already checked it in. Guess I can try to change the combine
> > > splitters
> > > (can it wait till Monday?) so that they remove the masking when splitting
> > > the insn into two, so that the pre-reload splitters aren't involved.
> >
> > No, I didn't want to burden you with the additional task - the patch
> > is OK as it is. I was just thinking out loud, as I remembered that
> > changing bt patterns to combine splitter regressed one testcase. IIRC
> > combination of two insns blocked better combination of three insns, or
> > something like that.
> >
> > > To turn those pre-reload define_insn_and_splits I'm afraid we'd indeed
> > > need combiner's changes, so that would need to be discussed with Segher
> > > first.
> >
> > Yes, that is the long-term plan. Segher CC'd.
>
> A splitter can *already* split to only one insn.
Unfortunately, the attached patch with the following testcase:
--cut here--
int test (int a, int b)
{
return a << (b & 31);
}
--cut here--
does not trigger the call to combine_split_insns. The reason in the
following condition:
/* If we were combining three insns and the result is a simple SET
with no ASM_OPERANDS that wasn't recognized, try to split it into two
insns. There are two ways to do this. It can be split using a
machine-specific method (like when you have an addition of a large
constant) or by combine in the function find_split_point. */
if (i1 && insn_code_number < 0 && GET_CODE (newpat) == SET
&& asm_noperands (newpat) < 0)
where i1 is null.
Uros.
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 21f0044179f..124495c1c4c 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -10883,50 +10883,35 @@
})
;; Avoid useless masking of count operand.
-(define_insn_and_split "*ashl<mode>3_mask"
+(define_split
[(set (match_operand:SWI48 0 "nonimmediate_operand")
(ashift:SWI48
(match_operand:SWI48 1 "nonimmediate_operand")
(subreg:QI
(and:SI
- (match_operand:SI 2 "register_operand" "c,r")
- (match_operand:SI 3 "const_int_operand")) 0)))
- (clobber (reg:CC FLAGS_REG))]
+ (match_operand:SI 2 "register_operand")
+ (match_operand:SI 3 "const_int_operand")) 0)))]
"ix86_binary_operator_ok (ASHIFT, <MODE>mode, operands)
&& (INTVAL (operands[3]) & (GET_MODE_BITSIZE (<MODE>mode)-1))
- == GET_MODE_BITSIZE (<MODE>mode)-1
- && ix86_pre_reload_split ()"
- "#"
- "&& 1"
- [(parallel
- [(set (match_dup 0)
- (ashift:SWI48 (match_dup 1)
- (match_dup 2)))
- (clobber (reg:CC FLAGS_REG))])]
- "operands[2] = gen_lowpart (QImode, operands[2]);"
- [(set_attr "isa" "*,bmi2")])
+ == GET_MODE_BITSIZE (<MODE>mode)-1"
+ [(set (match_dup 0)
+ (ashift:SWI48 (match_dup 1)
+ (match_dup 2)))]
+ "operands[2] = gen_lowpart (QImode, operands[2]);")
-(define_insn_and_split "*ashl<mode>3_mask_1"
+(define_split
[(set (match_operand:SWI48 0 "nonimmediate_operand")
(ashift:SWI48
(match_operand:SWI48 1 "nonimmediate_operand")
(and:QI
- (match_operand:QI 2 "register_operand" "c,r")
- (match_operand:QI 3 "const_int_operand"))))
- (clobber (reg:CC FLAGS_REG))]
+ (match_operand:QI 2 "register_operand")
+ (match_operand:QI 3 "const_int_operand"))))]
"ix86_binary_operator_ok (ASHIFT, <MODE>mode, operands)
&& (INTVAL (operands[3]) & (GET_MODE_BITSIZE (<MODE>mode)-1))
- == GET_MODE_BITSIZE (<MODE>mode)-1
- && ix86_pre_reload_split ()"
- "#"
- "&& 1"
- [(parallel
- [(set (match_dup 0)
- (ashift:SWI48 (match_dup 1)
- (match_dup 2)))
- (clobber (reg:CC FLAGS_REG))])]
- ""
- [(set_attr "isa" "*,bmi2")])
+ == GET_MODE_BITSIZE (<MODE>mode)-1"
+ [(set (match_dup 0)
+ (ashift:SWI48 (match_dup 1)
+ (match_dup 2)))])
(define_insn "*bmi2_ashl<mode>3_1"
[(set (match_operand:SWI48 0 "register_operand" "=r")
@@ -11403,50 +11388,35 @@
"ix86_expand_binary_operator (<CODE>, <MODE>mode, operands); DONE;")
;; Avoid useless masking of count operand.
-(define_insn_and_split "*<shift_insn><mode>3_mask"
+(define_split
[(set (match_operand:SWI48 0 "nonimmediate_operand")
(any_shiftrt:SWI48
(match_operand:SWI48 1 "nonimmediate_operand")
(subreg:QI
(and:SI
- (match_operand:SI 2 "register_operand" "c,r")
- (match_operand:SI 3 "const_int_operand")) 0)))
- (clobber (reg:CC FLAGS_REG))]
+ (match_operand:SI 2 "register_operand")
+ (match_operand:SI 3 "const_int_operand")) 0)))]
"ix86_binary_operator_ok (<CODE>, <MODE>mode, operands)
&& (INTVAL (operands[3]) & (GET_MODE_BITSIZE (<MODE>mode)-1))
- == GET_MODE_BITSIZE (<MODE>mode)-1
- && ix86_pre_reload_split ()"
- "#"
- "&& 1"
- [(parallel
- [(set (match_dup 0)
- (any_shiftrt:SWI48 (match_dup 1)
- (match_dup 2)))
- (clobber (reg:CC FLAGS_REG))])]
- "operands[2] = gen_lowpart (QImode, operands[2]);"
- [(set_attr "isa" "*,bmi2")])
+ == GET_MODE_BITSIZE (<MODE>mode)-1"
+ [(set (match_dup 0)
+ (any_shiftrt:SWI48 (match_dup 1)
+ (match_dup 2)))]
+ "operands[2] = gen_lowpart (QImode, operands[2]);")
-(define_insn_and_split "*<shift_insn><mode>3_mask_1"
+(define_split
[(set (match_operand:SWI48 0 "nonimmediate_operand")
(any_shiftrt:SWI48
(match_operand:SWI48 1 "nonimmediate_operand")
(and:QI
- (match_operand:QI 2 "register_operand" "c,r")
- (match_operand:QI 3 "const_int_operand"))))
- (clobber (reg:CC FLAGS_REG))]
+ (match_operand:QI 2 "register_operand")
+ (match_operand:QI 3 "const_int_operand"))))]
"ix86_binary_operator_ok (<CODE>, <MODE>mode, operands)
&& (INTVAL (operands[3]) & (GET_MODE_BITSIZE (<MODE>mode)-1))
- == GET_MODE_BITSIZE (<MODE>mode)-1
- && ix86_pre_reload_split ()"
- "#"
- "&& 1"
- [(parallel
- [(set (match_dup 0)
- (any_shiftrt:SWI48 (match_dup 1)
- (match_dup 2)))
- (clobber (reg:CC FLAGS_REG))])]
- ""
- [(set_attr "isa" "*,bmi2")])
+ == GET_MODE_BITSIZE (<MODE>mode)-1"
+ [(set (match_dup 0)
+ (any_shiftrt:SWI48 (match_dup 1)
+ (match_dup 2)))])
(define_insn_and_split "*<shift_insn><dwi>3_doubleword_mask"
[(set (match_operand:<DWI> 0 "register_operand")
