> On 31 Aug 2017, at 7:20 AM, Matthew Fortune <matthew.fort...@imgtec.com> > wrote: > > Jeff Law <l...@redhat.com> writes: >> On 08/30/2017 06:52 AM, Richard Biener wrote: >>> On Wed, Aug 30, 2017 at 11:53 AM, Michael Clark <michaeljcl...@mac.com> >>> wrote: >>>> >>>>> On 30 Aug 2017, at 9:43 PM, Michael Clark <michaeljcl...@mac.com> wrote: >>>>> >>>>>>> diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c >>>>>>> index ce632ae..25dd70f 100644 >>>>>>> --- a/gcc/simplify-rtx.c >>>>>>> +++ b/gcc/simplify-rtx.c >>>>>>> @@ -1503,6 +1503,10 @@ simplify_unary_operation_1 (enum rtx_code code, >>>>>>> machine_mode >> mode, rtx op) >>>>>>> /* (sign_extend:M (lshiftrt:N <X> (const_int I))) is better as >>>>>>> (zero_extend:M (lshiftrt:N <X> (const_int I))) if I is not 0. */ >>>>>>> if (GET_CODE (op) == LSHIFTRT >>>>>>> +#if defined(POINTERS_EXTEND_UNSIGNED) >>>>>>> + /* we skip this optimisation if pointers naturally extend signed >>>>>>> */ >>>>>>> + && POINTERS_EXTEND_UNSIGNED >>>>>>> +#endif >>>>>>> && CONST_INT_P (XEXP (op, 1)) >>>>>>> && XEXP (op, 1) != const0_rtx) >>>>>>> return simplify_gen_unary (ZERO_EXTEND, mode, op, GET_MODE (op)); >>>>>> >>>>>> Is it just me or does this miss a || mode != Pmode || GET_MODE (op) != >>>>>> ptr_mode >>>>>> check? Note the comment says exactly the opposite as the transform... >>>>>> >>>>>> I’m not even sure why this simplification is correct in the first place?! >>>>> >>>>> I hope you are not confusing my use of POINTERS_EXTEND_UNSIGNED as a >>>>> proxy for the >> property that defines whether sub width operations sign-extend to the full >> width of the >> register vs zero extend. Are you taking about our added comment? >>> >>> I'm talking about using POINTERS_EXTEND_UNSIGNED for sth that looks >>> unrelated (and that has no easy way to be queried as you noted). >> Right. I was going to make the same observation. I can't see how >> POINTER_EXTEND_UNSIGNED plays a significant role here. >> >> MIPS has similar properties and my recollection is they did some >> interesting tricks in the target files to fold the extension back into >> the arithmetic insns (beyond the usual LOAD_EXTEND_OP, >> WORD_REGISTER_OPERATIONS, TRULY_NOOP_TRUNCATION, and PROMOTE_MODE stuff). > > If there is a condition to add I would have expected it to be based around > WORD_REGISTER_OPERATIONS etc too.
Okay. Thanks. To disable the right shift zero extend optimisation we can perhaps then look into an expression that uses WORD_REGISTER_OPERATIONS && LOAD_EXTEND_OP(mode) == SIGN_EXTEND. What ever semantically means, sub word operations on SIMode are sign-extended vs zero-extended. >> My recollection was they defined their key insns with match_operators >> that allowed the sign extension to occur in the arithmetic insns. BUt I >> don't see any evidence of that anymore. But I can distinctly remember >> discussing it with Ian and Meissner eons ago and its impact on reload in >> particular. > > I see that riscv has chosen to not allow ior/and/xor with SImode as named > patterns but instead just for combine to pick up. Given that the > architecture has almost all the same properties as MIPS I don't follow why > the SImode version is not allowed at expand time. MIPS relies on all SImode > values being in a canonical sign extended form at all points and can > therefore freely represent the dual (or rather no) mode operations, such as > comparisons and logical operations, as both SI and DI mode. This pretty much > solves the redundant sign extension issues. Just because the logical > operations only exist as '64-bit' operations in the 64-bit architecture does > not mean you can't tell GCC there are 32-bit versions as well; you just have > to present a logical view of the architecture rather than being overly > strict. LLVM for MIPS went through similar issues and I suspect RISCV will > hit the same kind of issues but the same solution was used and both 32bit > and 64-bit operations described with the same underlying instruction. > > Is there an architectural difference that means riscv can't do the same > thing? I don’t believe so. We were seeking guidance and in the process of discovering and figuring out how to best solve these issues as they cropped up. Changing the logical op patterns to match 32-bit operations on RV64 seems like a sensible approach. We’ll also have to look at the SLT/SLTU (Set Less Than/Set Less Than Unsigned) patterns as they don’t have 32-bit versions; and verify they are doing the right thing with respect to modes and sign extension… We’ll work on a revised patch and circle back… Thanks, Michael
