Richard Sandiford <rdsandif...@googlemail.com> writes: > Vladimir Makarov <vmaka...@redhat.com> writes: >> On 10/25/2012 05:45 AM, Richard Sandiford wrote: >>> Hi Vlad, >>> >>> As discussed in the reviews, one of the things that worried me was the >>> combination of: >>> >>> 1) the displacement fixup code in process_address assumes that the address >>> is exactly equal to BASE_LOC + INDEX_LOC + DISP (with null values >>> being equivalent to 0). >>> >>> 2) extract_address_regs allows (and has to allow) much more general forms >>> than that. >>> >>> 3) equiv_address_substitution folds base and index displacements >>> without checking the shape of the address. >>> >>> So the code from (1) could end up fixing a displacement created by (3) >>> even if the address isn't a simple BASE_LOC + INDEX_LOC + DISP. >>> >>> There's a similar problem with the relationship between INDEX_LOC >>> and INDEX_REG_LOC: we assume the scale is 1 unless INDEX is a MULT >>> of the index register. >>> >>> This patch adds two utility functions, one to test whether the address >>> is simple enough for the fixup code to handle, and one to see whether >>> the index scale is known. The latter handles ASHIFT as well as MULT, >>> since ASHIFT can be used in out-of-MEM address operands. >> That is really nice. >>> The condition: >>> >>> /* If the address already has a displacement, we can simply add the >>> new displacement to it. */ >>> if (ad->disp_loc) >>> return true; >>> >>> might leave a bit of a hole, but the asserts you added to >>> extract_address_regs should mean that disp_loc really is a >>> displacement (thanks for those btw). >> I am not sure about it. UNSPEC is also treated as a displacement. I >> don't think it can be combined. > > OK. As it happens, the patch I just sent (mid-air collision, sorry) > deals with those UNSPECs in a different way, so maybe I should hold > off and see where that discussion goes.
Sorry to follow up on myself, but I changed my mind. As far as SVN history goes, I think this stage should be committed as a separate patch, so... >>> In principle, we could apply a base or index displacement even in cases >>> that process_address can't fix up, test whether the result is valid, >>> and revert to the normal behaviour of reloading the full base or index >>> value if not. That's not going to be useful for x86 (or for MIPS :-)) >>> and I'm not planning to do it, but the valid_address_p patch I just sent >>> ought to help. >>> >>> Tested on x86_64-linux-gnu. Also tested by making sure that there >>> were no changes in assembly output for a set of gcc .ii files >>> (i.e. these extra checks didn't affect the code on x86_64 at least). >>> OK to install? >> Probably we should check UNSPEC too. Otherwise, it looks ok. Thanks, >> Richard. ...applied with that change after testing on x86_64-linux-gnu, thanks. Richard
