[PATCH] x86: Provide expanders for truncdisi2 and friends.

[PATCH] x86: Provide expanders for truncdisi2 and friends.

[Roger Sayle]

Even by my standards, this is an odd patch.  This adds expanders to
i386.md requesting that integer truncations be represented in RTL
using SUBREGs.  This exactly matches the (current) default behaviour
when TARGET_TRULY_NOOP_TRUNCATION is undefined.  Hence this patch
is mostly for documentation/teaching purposes, so that i386.md is a
template or role model for the patterns that a backend should provide.

As explained in my earlier post, defining TARGET_TRULY_NOOP_TRUNCATION
to always return false in the i386/x86_64 backend, results in 603
additional unexpected failures.  Adding these expanders avoids the
majority (412) of those regressions.

I'm not proposing that i386/x86_64 redefine TARGET_TRULY_NOOP_TRUNCATION
but these placeholder expanders may provide the backend the flexibility
of that option in the future, but it also helps to test some less frequently
invoked corners of the middle-end.

This patch has been tested on x86_64-pc-linux-gnu with a full "make
bootstrap" and "make -k check" with no new failures, indeed there
are (should be) absolutely no code changes with this patch.

Might these changes be of interest?  If not, at least this patch
is now archived on gcc-patches for future generations.


2020-07-12  Roger Sayle  <roger@nextmovesoftware.com>

gcc/ChangeLog
	* config/i386/i386.md (truncdi<SWI124>2, truncsi<SWI12>2,
	trunchiqi2): New expanders to make the intended representation
	of scalar integer truncations explicit.

Thoughts?
Roger
--
Roger Sayle
NextMove Software
Cambridge, UK

Re: [PATCH] x86: Provide expanders for truncdisi2 and friends.

[Richard Biener]

On Sun, Jul 12, 2020 at 1:29 AM Roger Sayle <roger@nextmovesoftware.com> wrote:
>
>
> Even by my standards, this is an odd patch.  This adds expanders to
> i386.md requesting that integer truncations be represented in RTL
> using SUBREGs.  This exactly matches the (current) default behaviour
> when TARGET_TRULY_NOOP_TRUNCATION is undefined.  Hence this patch
> is mostly for documentation/teaching purposes, so that i386.md is a
> template or role model for the patterns that a backend should provide.
>
> As explained in my earlier post, defining TARGET_TRULY_NOOP_TRUNCATION
> to always return false in the i386/x86_64 backend, results in 603
> additional unexpected failures.  Adding these expanders avoids the
> majority (412) of those regressions.
>
> I'm not proposing that i386/x86_64 redefine TARGET_TRULY_NOOP_TRUNCATION
> but these placeholder expanders may provide the backend the flexibility
> of that option in the future, but it also helps to test some less frequently
> invoked corners of the middle-end.
>
> This patch has been tested on x86_64-pc-linux-gnu with a full "make
> bootstrap" and "make -k check" with no new failures, indeed there
> are (should be) absolutely no code changes with this patch.
>
> Might these changes be of interest?  If not, at least this patch
> is now archived on gcc-patches for future generations.

It seems to be improving TARGET_TRULY_NOOP_TRUNCATION documentation
might be useful here - from what it says it suggests to me it applies to
hardregs only and tells for example whether there are lower than word_mode
precision registers like on x86_64 %al?  Because for pseudos there's
always the requirement to use subregs and doing (reg:SI 42) and (reg:QI 42)
accesses to the same pseudo is invalid(?).

The only user (after your patch) of this hook is in function.c for the
function parameter setup btw. and I wonder if there's other hooks
for the RA for example that provide duplicate functionality.

Richard.

>
> 2020-07-12  Roger Sayle  <roger@nextmovesoftware.com>
>
> gcc/ChangeLog
>         * config/i386/i386.md (truncdi<SWI124>2, truncsi<SWI12>2,
>         trunchiqi2): New expanders to make the intended representation
>         of scalar integer truncations explicit.
>
> Thoughts?
> Roger
> --
> Roger Sayle
> NextMove Software
> Cambridge, UK
>
>

RE: [PATCH] x86: Provide expanders for truncdisi2 and friends.

[Roger Sayle]

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


Hi Richard,

> It seems to be improving TARGET_TRULY_NOOP_TRUNCATION documentation might be useful here.

This is an excellent suggestion.  How about the following/attached:

2020-07-13  Roger Sayle  <roger@nextmovesoftware.com>

gcc/ChangeLog:
	* doc/tm.texi (TARGET_TRULY_NOOP_TRUNCATION): Clarify that targets
	that (sometimes) return false, indicating SUBREGs shouldn't be
	used, also need to provide a trunc?i?i2 optab that performs this
	truncation.

> The only user (after your patch) of this hook is in function.c for the function parameter setup btw.

The targetm.truly_noop_truncation in assign_parm_setup_block is the last place that calls this
hook directly (with sizes), but the majority of uses go via TRULY_NOOP_TRUNCATION_MODES_P
as defined in machmode.h.

I'll prepare a patch to switch function.c to use TRULY_NOOP_TRUNCATION_MODE_P so that we
are consistent throughout the compiler.  In theory, this hook could then be changed to take modes
instead of (poly_unit64) sizes, but that clean-up might be tricky without access to the affected
platforms.

The hard register semantics that you're referring to are related to TARGET_MODES_TIEABLE_P,
which is documented to have interesting interactions with TARGET_TRULY_NOOP_TRUNCATION.

Is the above documentation change Ok for mainline?

Thanks,
Roger
--


[-- Attachment #2: patche.txt --]
[-- Type: text/plain, Size: 831 bytes --]

diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 6e7d9dc54a9..41b9e10c856 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -11122,7 +11122,9 @@ This hook returns true if it is safe to ``convert'' a value of
 @var{inprec} bits to one of @var{outprec} bits (where @var{outprec} is
 smaller than @var{inprec}) by merely operating on it as if it had only
 @var{outprec} bits.  The default returns true unconditionally, which
-is correct for most machines.
+is correct for most machines.  When @code{TARGET_TRULY_NOOP_TRUNCATION}
+returns false, the machine description should provide a @code{trunc}
+optab to specify the RTL that performs the required truncation.
 
 If @code{TARGET_MODES_TIEABLE_P} returns false for a pair of modes,
 suboptimal code can result if this hook returns true for the corresponding

Re: [PATCH] x86: Provide expanders for truncdisi2 and friends.

[Richard Biener]

On Mon, Jul 13, 2020 at 4:50 PM Roger Sayle <roger@nextmovesoftware.com> wrote:
>
>
> Hi Richard,
>
> > It seems to be improving TARGET_TRULY_NOOP_TRUNCATION documentation might be useful here.
>
> This is an excellent suggestion.  How about the following/attached:
>
> 2020-07-13  Roger Sayle  <roger@nextmovesoftware.com>
>
> gcc/ChangeLog:
>         * doc/tm.texi (TARGET_TRULY_NOOP_TRUNCATION): Clarify that targets
>         that (sometimes) return false, indicating SUBREGs shouldn't be
>         used, also need to provide a trunc?i?i2 optab that performs this
>         truncation.
>
> > The only user (after your patch) of this hook is in function.c for the function parameter setup btw.
>
> The targetm.truly_noop_truncation in assign_parm_setup_block is the last place that calls this
> hook directly (with sizes), but the majority of uses go via TRULY_NOOP_TRUNCATION_MODES_P
> as defined in machmode.h.
>
> I'll prepare a patch to switch function.c to use TRULY_NOOP_TRUNCATION_MODE_P so that we
> are consistent throughout the compiler.  In theory, this hook could then be changed to take modes
> instead of (poly_unit64) sizes, but that clean-up might be tricky without access to the affected
> platforms.
>
> The hard register semantics that you're referring to are related to TARGET_MODES_TIEABLE_P,
> which is documented to have interesting interactions with TARGET_TRULY_NOOP_TRUNCATION.
>
> Is the above documentation change Ok for mainline?

OK.

Thanks,
Richard.

> Thanks,
> Roger
> --
>

RE: [PATCH] x86: Provide expanders for truncdisi2 and friends.

[Roger Sayle]

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


Hi Richard,

Many thanks.  As promised, here's a clean-up patch that removes the last direct
call to targetm.truly_noop_truncation from the middle-end, allowing this hook 
at some point in the future to take modes instead of sizes.

middle-end: Prefer TRULY_NOOP_TRUNCATION_MODES_P over raw target hook.

This patch has been tested with "make bootstrap" and "make -k check" on
x86_64-pc-linux-gnu with no regressions.

2020-07-16  Roger Sayle  <roger@nextmovesoftware.com>

gcc/ChangeLog
        * function.c (assign_parm_setup_block): Use the macro
        TRULY_NOOP_TRUNCATION_MODES_P instead of calling
        targetm.truly_noop_truncation directly.

Ok for mainline?

Thanks again,
Roger
--

-----Original Message-----
From: Richard Biener <richard.guenther@gmail.com> 
Sent: 14 July 2020 15:17
To: Roger Sayle <roger@nextmovesoftware.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] x86: Provide expanders for truncdisi2 and friends.

On Mon, Jul 13, 2020 at 4:50 PM Roger Sayle <roger@nextmovesoftware.com> wrote:
> > It seems to be improving TARGET_TRULY_NOOP_TRUNCATION documentation might be useful here.
> This is an excellent suggestion.  How about the following/attached...
>
> > The only user (after your patch) of this hook is in function.c for the function parameter setup btw.
>
> The targetm.truly_noop_truncation in assign_parm_setup_block is the 
> last place that calls this hook directly (with sizes), but the 
> majority of uses go via TRULY_NOOP_TRUNCATION_MODES_P as defined in machmode.h.
>
> I'll prepare a patch to switch function.c to use 
> TRULY_NOOP_TRUNCATION_MODE_P so that we are consistent throughout the 
> compiler.  In theory, this hook could then be changed to take modes 
> instead of (poly_unit64) sizes, but that clean-up might be tricky without access to the affected platforms.
>
> Is the above documentation change Ok for mainline?
OK.

Thanks,
Richard.

> Thanks,
> Roger
> --
>

[-- Attachment #2: patchb2.txt --]
[-- Type: text/plain, Size: 590 bytes --]

diff --git a/gcc/function.c b/gcc/function.c
index 9eee9b5..cdfcc4b 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -3013,8 +3013,8 @@ assign_parm_setup_block (struct assign_parm_data_all *all,
 		 to the value directly in mode MODE, otherwise we must
 		 start with the register in word_mode and explicitly
 		 convert it.  */
-	      if (targetm.truly_noop_truncation (size * BITS_PER_UNIT,
-						 BITS_PER_WORD))
+	      if (mode == word_mode
+		  || TRULY_NOOP_TRUNCATION_MODES_P (mode, word_mode))
 		reg = gen_rtx_REG (mode, REGNO (entry_parm));
 	      else
 		{

Re: [PATCH] x86: Provide expanders for truncdisi2 and friends.

[Richard Biener]

On Thu, Jul 16, 2020 at 1:07 PM Roger Sayle <roger@nextmovesoftware.com> wrote:
>
>
> Hi Richard,
>
> Many thanks.  As promised, here's a clean-up patch that removes the last direct
> call to targetm.truly_noop_truncation from the middle-end, allowing this hook
> at some point in the future to take modes instead of sizes.
>
> middle-end: Prefer TRULY_NOOP_TRUNCATION_MODES_P over raw target hook.
>
> This patch has been tested with "make bootstrap" and "make -k check" on
> x86_64-pc-linux-gnu with no regressions.

OK.

Richard.

> 2020-07-16  Roger Sayle  <roger@nextmovesoftware.com>
>
> gcc/ChangeLog
>         * function.c (assign_parm_setup_block): Use the macro
>         TRULY_NOOP_TRUNCATION_MODES_P instead of calling
>         targetm.truly_noop_truncation directly.
>
> Ok for mainline?
>
> Thanks again,
> Roger
> --
>
> -----Original Message-----
> From: Richard Biener <richard.guenther@gmail.com>
> Sent: 14 July 2020 15:17
> To: Roger Sayle <roger@nextmovesoftware.com>
> Cc: GCC Patches <gcc-patches@gcc.gnu.org>
> Subject: Re: [PATCH] x86: Provide expanders for truncdisi2 and friends.
>
> On Mon, Jul 13, 2020 at 4:50 PM Roger Sayle <roger@nextmovesoftware.com> wrote:
> > > It seems to be improving TARGET_TRULY_NOOP_TRUNCATION documentation might be useful here.
> > This is an excellent suggestion.  How about the following/attached...
> >
> > > The only user (after your patch) of this hook is in function.c for the function parameter setup btw.
> >
> > The targetm.truly_noop_truncation in assign_parm_setup_block is the
> > last place that calls this hook directly (with sizes), but the
> > majority of uses go via TRULY_NOOP_TRUNCATION_MODES_P as defined in machmode.h.
> >
> > I'll prepare a patch to switch function.c to use
> > TRULY_NOOP_TRUNCATION_MODE_P so that we are consistent throughout the
> > compiler.  In theory, this hook could then be changed to take modes
> > instead of (poly_unit64) sizes, but that clean-up might be tricky without access to the affected platforms.
> >
> > Is the above documentation change Ok for mainline?
> OK.
>
> Thanks,
> Richard.
>
> > Thanks,
> > Roger
> > --
> >

RE: [PATCH] x86: Provide expanders for truncdisi2 and friends.

[Roger Sayle]

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


Doh! With the attachment.


-----Original Message-----
From: Roger Sayle <roger@nextmovesoftware.com> 
Sent: 12 July 2020 00:29
To: 'gcc-patches@gcc.gnu.org' <gcc-patches@gcc.gnu.org>
Subject: [PATCH] x86: Provide expanders for truncdisi2 and friends.


Even by my standards, this is an odd patch.  This adds expanders to i386.md
requesting that integer truncations be represented in RTL using SUBREGs.
This exactly matches the (current) default behaviour when
TARGET_TRULY_NOOP_TRUNCATION is undefined.  Hence this patch is mostly for
documentation/teaching purposes, so that i386.md is a template or role model
for the patterns that a backend should provide.

As explained in my earlier post, defining TARGET_TRULY_NOOP_TRUNCATION to
always return false in the i386/x86_64 backend, results in 603 additional
unexpected failures.  Adding these expanders avoids the majority (412) of
those regressions.

I'm not proposing that i386/x86_64 redefine TARGET_TRULY_NOOP_TRUNCATION but
these placeholder expanders may provide the backend the flexibility of that
option in the future, but it also helps to test some less frequently invoked
corners of the middle-end.

This patch has been tested on x86_64-pc-linux-gnu with a full "make
bootstrap" and "make -k check" with no new failures, indeed there are
(should be) absolutely no code changes with this patch.

Might these changes be of interest?  If not, at least this patch is now
archived on gcc-patches for future generations.


2020-07-12  Roger Sayle  <roger@nextmovesoftware.com>

gcc/ChangeLog
	* config/i386/i386.md (truncdi<SWI124>2, truncsi<SWI12>2,
	trunchiqi2): New expanders to make the intended representation
	of scalar integer truncations explicit.

Thoughts?
Roger
--
Roger Sayle
NextMove Software
Cambridge, UK


[-- Attachment #2: patchd2.txt --]
[-- Type: text/plain, Size: 1004 bytes --]

diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index d0ecd9e..4929c05 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -5144,6 +5144,39 @@
     }
 })
 \f
+;; Integer conversions
+
+(define_expand "truncdi<SWI124:mode>2"
+  [(set (match_operand:SWI124 0 "register_operand")
+	(subreg:SWI124
+	  (match_operand:DI 1 "register_operand")
+	  0))]
+  ""
+{
+  operands[1] = force_reg (DImode, operands[1]);
+})
+
+(define_expand "truncsi<SWI12:mode>2"
+  [(set (match_operand:SWI12 0 "register_operand")
+	(subreg:SWI12
+	  (match_operand:SI 1 "register_operand")
+	  0))]
+  ""
+{
+  operands[1] = force_reg (SImode, operands[1]);
+})
+
+(define_expand "trunchiqi2"
+  [(set (match_operand:QI 0 "register_operand")
+	(subreg:QI
+	  (match_operand:HI 1 "register_operand")
+	  0))]
+  ""
+{
+  operands[1] = force_reg (HImode, operands[1]);
+})
+
+\f
 ;; Load effective address instructions
 
 (define_insn_and_split "*lea<mode>"