On Tue, Apr 3, 2018 at 12:48 AM, Jim Wilson <j...@sifive.com> wrote:
> On Fri, Mar 23, 2018 at 5:33 AM, Richard Biener
> <richard.guent...@gmail.com> wrote:
>> I'm leaving the "simple" combiner patch to review by others
>> but for RISC-V you could simply #define SHIFT_COUNT_TRUNCATED to zero
>> to fix the issue.  Then add patterns if it turns out to be required
>> to avoid regressions.  For example x86 has
>
> I checked in a patch to do this.  It took me quite a few iterations to
> work out how to do it without regressions, but it looks OK in the end.
> 6 additional patterns and changing the shift count mode to QImode in
> all existing shift patterns and I get effectively the same code as
> before with SHIFT_COUNT_TRUNCATED turned off.  With DImode shift
> counts, I ran into problems with unnecessary sign/zero extensions from
> SImode for shift counts.  With SImode shift counts, I ran into
> problems with unnecessary truncations from DImode.  Using QImode shift
> counts avoided both of those problems.
>
> That leaves the other targets still using SHIFT_COUNT_TRUNCATED as
> potentially broken.  I count 16 of them.  Though this particular
> testcase might not be triggerable on most of them, since it requires
> shift patterns for two different modes, and most 32-bit targets
> probably only have one mode that shift is supported for.

Thanks a lot for going this route for fixing.  I only count the sparc
as important platform unconditionally defining it to 1, the rest is
weird embedded targets or targest with at least one mode where
it is set to zero.  OK, maybe add mips to the list of targets.

So I'd be happy to just kill it off early during GCC 9 development...

But I guess I proposed this before and people weren't entirely
happy.

Richard.

> Jim

Reply via email to