Re: [PATCH] x86: Provide expanders for truncdisi2 and friends.
On Thu, Jul 16, 2020 at 1:07 PM Roger Sayle 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 > > 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 > Sent: 14 July 2020 15:17 > To: Roger Sayle > Cc: GCC Patches > Subject: Re: [PATCH] x86: Provide expanders for truncdisi2 and friends. > > On Mon, Jul 13, 2020 at 4:50 PM Roger Sayle > 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.
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
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
Sent: 14 July 2020 15:17
To: Roger Sayle
Cc: GCC Patches
Subject: Re: [PATCH] x86: Provide expanders for truncdisi2 and friends.
On Mon, Jul 13, 2020 at 4:50 PM Roger Sayle 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
> --
>
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.
On Mon, Jul 13, 2020 at 4:50 PM Roger Sayle 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 > > 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.
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
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
--
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.
On Sun, Jul 12, 2020 at 1:29 AM Roger Sayle 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 > > gcc/ChangeLog > * config/i386/i386.md (truncdi2, truncsi2, > 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.
Doh! With the attachment.
-Original Message-
From: Roger Sayle
Sent: 12 July 2020 00:29
To: '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
gcc/ChangeLog
* config/i386/i386.md (truncdi2, truncsi2,
trunchiqi2): New expanders to make the intended representation
of scalar integer truncations explicit.
Thoughts?
Roger
--
Roger Sayle
NextMove Software
Cambridge, UK
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 @@
}
})
+;; Integer conversions
+
+(define_expand "truncdi2"
+ [(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 "truncsi2"
+ [(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]);
+})
+
+
;; Load effective address instructions
(define_insn_and_split "*lea"
[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 gcc/ChangeLog * config/i386/i386.md (truncdi2, truncsi2, trunchiqi2): New expanders to make the intended representation of scalar integer truncations explicit. Thoughts? Roger -- Roger Sayle NextMove Software Cambridge, UK
