[prev in list] [next in list] [prev in thread] [next in thread] 

List:       gcc-patches
Subject:    Re: [PATCH] Define TARGET_TRULY_NOOP_TRUNCATION to false.
From:       Tom de Vries <tdevries () suse ! de>
Date:       2020-07-31 17:17:39
Message-ID: d31065bc-0c57-d858-4bf7-e37400a3158b () suse ! de
[Download RAW message or body]

On 7/16/20 12:02 PM, Roger Sayle wrote:
> 
> Many thanks to Richard Biener for approving the midde-end
> patch that cleared the way for this one.  This nvptx patch
> defines the target hook TARGET_TRULY_NOOP_TRUNCATION to
> false, indicating that integer truncations require explicit
> instructions.  nvptx.c already defines TARGET_MODES_TIEABLE_P
> and TARGET_CAN_CHANGE_MODE_CLASS to false, and as (previously)
> documented that may require TARGET_TRULY_NOOP_TRUNCATION to
> be defined likewise.
> 
> This patch decreases the number of unexpected failures in
> the testsuite by 10, and increases the number of expected
> passes by 4, including these previous FAILs/ICEs:
> gcc.c-torture/compile/opout.c
> gcc.dg/torture/pr79125.c
> gcc.dg/tree-ssa/pr92085-1.c
> 

That's great :)

I did an investigation with a test program doing a truncation (
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96401 ) to get the context
clear in my head.

The patch looks good to me, on the basis of the rationale and the fact
that it fixes long-standing ICEs.

[ FWIW, my investigation showed that we may have a benefit from
TARGET_MODES_TIEABLE_P == true, so perhaps part of that rationale might
disappear, but I don't think that's relevant at this point. ]

> Unfortunately there is one testsuite failure that used to
> pass gcc.target/nvptx/v2si-cvt.c, but this isn't an ICE or
> incorrect code.  As explained in this test, the scan-assembler
> already isn't testing what it should.  Given that there are
> several (nvptx) patches pending review that affect code
> generation of this example, and (perhaps) more on the way,
> I propose letting this test FAIL for now until the dust
> settles and it becomes clear what instruction(s) should be
> generated (certainly not a cvt.u32.u32).
> 

I've filed the regression at
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96403.

> This patch has been tested on nvptx-none hosted on
> x86_64-pc-linux-gnu with "make" and "make check" with
> fewer ICEs and no wrong code regressions.
> Ok for mainline?
> 

Committed to trunk, with the FAILing tests replaced by a mention of the
regression PR.

Thanks,
- Tom
[prev in list] [next in list] [prev in thread] [next in thread]