[rtl] Harden 'set_noop_p' for non-constant selectors [PR94279]

Richard Sandiford richard.sandiford@arm.com
Wed Apr 22 17:23:24 GMT 2020


Andrew Stubbs <ams@codesourcery.com> writes:
> On 22/04/2020 17:43, Thomas Schwinge wrote:
>> In <https://gcc.gnu.org/PR94279> "[amdgcn] internal compiler error: RTL
>> check: expected code 'const_int', have 'reg' in rtx_to_poly_int64, at
>> rtl.h:2379", we recently found that that it's wrong to expect constant
>> selectors, at least in the current code and its usage context.  (Thanks,
>> Richard Biener for the guidance!)  Not too many actually, but of course,
>> this code has seen some changes since 2013-12-04 (for example, r261530
>> "Use poly_int rtx accessors instead of hwi accessors"), and also the
>> context may have changed that it's being used in -- so, I'm not sure
>> whether the original code (as quoted above) is actually buggy already,
>> but it already does contain the pattern that 'INTVAL' is used on
>> something without making sure that we're actually dealing with a constant
>> selector.  (Has that maybe have been an impossible scenario back then?)
>
> I think it was impossible. See 
> https://gcc.gnu.org/legacy-ml/gcc-patches/2018-09/msg00273.html

Ah!  Thanks for the link.

>> Then, should this also be backported to release branches?  GCC 9: same
>> patch as for master branch.  GCC 8: pre poly_int, so only need to guard
>> 'INTVAL' (by 'CONST_INT_P', right?).  Or, is that not worth it, given
>> that nobody found this to be a problem until now (as far as I know),
>> and/or it's maybe really specific to (or, exposed by) AMD GCN's vector
>> instructions?  (For AMD GCN offloading, we only care about master
>> branch.)
>
> I don't think it's needed prior to GCC 9, and then only for amdgcn which 
> was probably not widely used.

Based on that, OK for master and GCC 9.

Thanks,
Richard


More information about the Gcc-patches mailing list