On Tue, Sep 24, 2013 at 8:20 AM, bin.cheng <bin.ch...@arm.com> wrote: > > >> -----Original Message----- >> From: Richard Biener [mailto:richard.guent...@gmail.com] >> Sent: Monday, September 23, 2013 8:08 PM >> To: Bin Cheng >> Cc: GCC Patches >> Subject: Re: [PATCH]Construct canonical scaled address expression in IVOPT >> >> On Fri, Sep 20, 2013 at 12:00 PM, bin.cheng <bin.ch...@arm.com> wrote: >> > Hi, >> > For now IVOPT constructs scaled address expression in the form of >> > "scaled*index" and checks whether backend supports it. The problem is >> > the address expression is invalid on ARM, causing scaled expression >> > disabled in IVOPT on ARM. This patch fixes the IVOPT part by >> > constructing rtl address expression like "index*scaled+base". >> >> - addr = gen_rtx_fmt_ee (MULT, address_mode, reg1, NULL_RTX); >> + /* Construct address expression in the canonical form of >> + "base+index*scale" and call memory_address_addr_space_p to see >> whether >> + it is allowed by target processors. */ >> + index = gen_rtx_fmt_ee (MULT, address_mode, reg1, NULL_RTX); >> for (i = -MAX_RATIO; i <= MAX_RATIO; i++) >> { >> - XEXP (addr, 1) = gen_int_mode (i, address_mode); >> + if (i == 1) >> + continue; >> + >> + XEXP (index, 1) = gen_int_mode (i, address_mode); addr = >> + gen_rtx_fmt_ee (PLUS, address_mode, index, reg1); >> if (memory_address_addr_space_p (mode, addr, as)) >> bitmap_set_bit (valid_mult, i + MAX_RATIO); >> >> so you now build reg1*scale+reg1 - which checks if offset and scale work > at >> the same time (and with the same value - given this is really > reg1*(scale+1)). >> It might be that this was implicitely assumed (or that no targets allow > scale >> but no offset), but it's a change that requires audit of behavior on more >> targets. > So literally, function multiplier_allowed_in_address_p should check whether > multiplier is allowed in any kind of addressing mode, like [reg*scale + > offset] and [reg*scale + reg].
Or even [reg*scale] (not sure about that). But yes, at least reg*scale + offset and reg*scale + reg. > Apparently it's infeasible to check every > possibility for each architecture, is it ok we at least check "index", then > "addr" if "index" is failed? By "any kind of addressing modes", I mean > modes supported by function get_address_cost, i.e., in form of "[base] + > [off] + [var] + (reg|reg*scale)". I suppose so, but it of course depends on what IVOPTs uses the answer for in the end. Appearantly it doesn't distinguish between the various cases even though TARGET_MEM_REF does support all the variants in question (reg * scale, reg * scale + reg, reg * scale + const, reg * scale + reg + const). So the better answer may be to teach the costs about the differences? >> The above also builds more RTX waste which you can fix by re-using the > PLUS >> by building it up-front similar to the multiplication. You also miss the > Yes, this can be fixed. > >> opportunity to have scale == 1 denote as to whether reg1 + reg2 is valid. > I >> would expect that many targets support reg1 * scale + constant-offset but >> not many reg1 * scale + reg2. > I thought scale==1 is unnecessary because the addressing mode degrades into > "reg" or "reg+reg". Moreover, calls of multiplier_allowed_in_address_p in > both get_address_cost and get_computation_cost_at have scale other than 1. Ok. >> >> So no, the helper now checks sth completely different. What's the problem >> with arm supporting reg1 * scale? Why shouldn't it being able to handle > the >> implicit zero offset? > > As Richard clarified, ARM does not support scaled addressing mode without > base register. I see. Richard.