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
