On 5/10/19 1:08 AM, Richard Biener wrote: > 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. Understood and agreed. So I think this argues that Robin should look at a different approach and we should start pushing ports away from SHIFT_COUNT_TRUNCATED a bit more aggressively.
Jeff
