On 7/2/20 3:10 PM, Roger Sayle wrote: > Hi Tom, > > Beware of doing something just because the default target hook does it that > way.
Well, what I'm saying is that if the default target hook doesn't assume tree_fits_uhwi_p (size), the safest solution is to do the same in the nvptx target hook. Thanks, - Tom > See https://gcc.gnu.org/pipermail/gcc-patches/2020-June/549128.html > which fixes PR middle-end/90597 by correcting default_vector_alignment to do > it the same way as proposed by this nvptx backend patch. > > Many thanks for pointing out that the nvptx problem is PR target/90932. > > Roger > -- > > -----Original Message----- > From: Tom de Vries <tdevr...@suse.de> > Sent: 02 July 2020 13:09 > To: Roger Sayle <ro...@nextmovesoftware.com>; gcc-patches@gcc.gnu.org > Subject: Re: [PATCH] nvptx: Fix ICE in nvptx_vector_alignment on > gcc.dg/attr-vector_size.c > > On 6/29/20 7:19 PM, Roger Sayle wrote: >> This patch addresses the ICE in gcc.dg/attr-vector_size.c during make >> -k check on nvptx-none. The actual ICE looks like: >> >> testsuite/gcc.dg/attr-vector_size.c:29:1: internal compiler error: in >> tree_to_shwi, at tree.c:7321 >> 0xf53bf2 tree_to_shwi(tree_node const*) >> ../../gcc/gcc/tree.c:7321 >> 0xff1969 nvptx_vector_alignment >> ../../gcc/gcc/config/nvptx/nvptx.c:5105^M >> >> The problem is that the caller has ensured that TYPE_SIZE(type) is >> representable as an unsigned HOST_WIDE_INT, but nvptx_vector_alignment >> is accessing it as a signed HOST_WIDE_INT which overflows in >> pathological conditions. Amongst those pathological conditions is >> that a TYPE_SIZE of zero can sometimes reach this function, prior to >> an error being emitted. Making sure the result is not less than the >> mode's alignment and not greater than BIGGEST_ALIGNMENT fixes the ICEs, and >> generates the expected compile-time error messages. >> >> Tested on --target=nvptx-none, with a "make" and "make check" which >> results in four fewer unexpected failures and three more expected passes. >> Ok for mainline? >> >> >> 2020-06-29 Roger Sayle <ro...@nextmovesoftware.com> >> >> gcc/ChangeLog: >> * config/nvptx/nvptx.c (nvptx_vector_alignment): Use tree_to_uhwi >> to access TYPE_SIZE (type). Return at least the mode's alignment. >> >> Thanks, >> Roger >> -- >> Roger Sayle >> NextMove Software >> Cambridge, UK >> >> diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c index >> e3e84df..bfad91b 100644 >> --- a/gcc/config/nvptx/nvptx.c >> +++ b/gcc/config/nvptx/nvptx.c >> @@ -5102,9 +5102,10 @@ static const struct attribute_spec >> nvptx_attribute_table[] = static HOST_WIDE_INT >> nvptx_vector_alignment (const_tree type) { >> - HOST_WIDE_INT align = tree_to_shwi (TYPE_SIZE (type)); >> - >> - return MIN (align, BIGGEST_ALIGNMENT); >> + unsigned HOST_WIDE_INT align = tree_to_uhwi (TYPE_SIZE (type)); > > In the default target hook, we test tree_fits_uhwi_p, so I prefer we do the > same. > >> + if (align > BIGGEST_ALIGNMENT) >> + return BIGGEST_ALIGNMENT; > > I prefer using MIN to make code easier to read. > >> + return MAX (align, GET_MODE_ALIGNMENT (TYPE_MODE (type))); >> } >> >> /* Indicate that INSN cannot be duplicated. */ > > Also, this is related to a PR, number included in commit log. > > As of yet untested updated patch attached. > > Thanks, > - Tom >
