Hi! On Wed, Sep 03, 2025 at 08:06:08PM +0530, Avinash Jayakar wrote: > On Tue, 2025-09-02 at 10:38 +0530, Surya Kumari Jangala wrote: > > > > > > > There is code in the routine expand_shift_1() that checks if the left > > shift can be implemented as a sequence of ADDs : > > > > /* Check whether its cheaper to implement a left shift by a > > constant > > bit count by a sequence of additions. */ > > if (code == LSHIFT_EXPR > > && CONST_INT_P (op1) > > && INTVAL (op1) > 0 > > && INTVAL (op1) < GET_MODE_PRECISION (scalar_mode) > > && INTVAL (op1) < MAX_BITS_PER_WORD > > && (shift_cost (speed, mode, INTVAL (op1)) > > > INTVAL (op1) * add_cost (speed, mode)) > > && shift_cost (speed, mode, INTVAL (op1)) != MAX_COST) > > > > We need to check why this is not working for powerpc. Can the costs > > be improved for powerpc to enable replacement of shift by add? > > > I checked the actual costs and looks like the shift_cost and add_cost > are the same(4).
Both are COST_N_INSNS (1), as expected. Both are just simple insns. Things start to get interesting when you get non-trivial sequences of instructions. The generic code works for all architectures, also those that aren't so very sane :-) > I do not see a target specific definition for these > costs. All such insns are recognised by some pattern in the machine description (that is rs6000.md and everything included into that), and then insn_cost is done in it (which via pattern_cost and set_src_cost would make pretty much every insn the same cost, but we get rs6000_insn_cost instead, which also just counts insns, but it is a bit more subtle too, it makes loads slightly more expensive to discourage them, and it makes multiplications a lot more expensive, and things like that). > All I see is this global struct "this_target_expmed" is > initialized with "default_target_expmed", where these costs are > defined. > Ideally in case of powerpc and in vector mode, the shift_cost should be > more than add_cost right, since we need to first splat the constant > operand into a register and then do the shift. I think this cost is not > accounted for. Various older (generic) things do not use insn_cost, but instead use rtx_cost. A very bad plan! This function estimates how expensive some RTX operation would be without knowing what insns will be created for it. I have modified various passes over the years to create some temporary RTL just to calculate the actual cost for using it. If you find any more that needs attention please let me know :-) Segher
