On Thu, May 9, 2019 at 6:00 PM Jeff Law <l...@redhat.com> wrote: > > On 5/9/19 5:52 AM, Robin Dapp wrote: > > Hi, > > > > while trying to improve s390 code generation for rotate and shift I > > noticed superfluous subregs for shift count operands. In our backend we > > already have quite cumbersome patterns that would need to be duplicated > > (or complicated further by more subst patterns) in order to get rid of > > the subregs. > > > > I had already finished all the patterns when I realized that > > SHIFT_COUNT_TRUNCATED and the target hook shift_truncation_mask already > > exist and could do what is needed without extra patterns. Just defining > > shift_truncation_mask was not enough though as most of the additional > > insns get introduced by combine. > > > > Event SHIFT_COUNT_TRUNCATED is no perfect match to what our hardware > > does because we always only consider the last 6 bits of a shift operand.> > > Despite all the warnings in the other backends, most notably > > SHIFT_COUNT_TRUNCATED being "discouraged" as mentioned in riscv.h, I > > wrote the attached tentative patch. It's a little ad-hoc, uses the > > SHIFT_COUNT_TRUNCATED paths only if shift_truncation_mask () != 0 and, > > instead of truncating via & (GET_MODE_BITSIZE (mode) - 1), it applies > > the mask returned by shift_truncation_mask. Doing so, usage of both > > "methods" actually reduces to a single way. > THe main reason it's discouraged is because some targets have insns > where the count would be truncated and others where it would not. So > for example normal shifts might truncate, but vector shifts might or > (mips) or shifts might truncate but bit tests do not (x86). > > I don't know enough about the s390 architecture to know if there's any > corner cases. You'd have to look at ever pattern in your machine > description with a shift and verify that it's going to DTRT if the count > hasn't been truncated. > > > It would really help if you could provide testcases which show the > suboptimal code and any analysis you've done.
The main reason I dislike SHIFT_COUNT_TRUNCATED is that it changes the meaning of the IL. We generally want these kind of things to be explicit. Richard. > > Jeff
